Skip to content

Pull Request #2

Open
OpalJellyfish wants to merge 55 commits into
vinitha910:masterfrom
OpalJellyfish:dev
Open

Pull Request #2
OpalJellyfish wants to merge 55 commits into
vinitha910:masterfrom
OpalJellyfish:dev

Conversation

@OpalJellyfish

@OpalJellyfish OpalJellyfish commented Apr 4, 2019

Copy link
Copy Markdown

Implemented Graph and Dijkstra

Even if I made some minor mistakes, please leave me comments about them! I really want to improve my C++ coding skills.

Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp
Comment thread dijkstra.cpp Outdated
Comment thread graph.cpp Outdated
Comment thread graph.cpp Outdated
Comment thread graph.cpp Outdated
Comment thread graph.cpp Outdated
Comment thread graph.cpp Outdated

@vinitha910 vinitha910 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Just very minor stuff. Also, be careful with where you put an extra line between code. The general rule of thumb is to group lines of code that are relevant to one another

Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.cpp Outdated
Comment thread dijkstra.h Outdated
Comment thread graph.cpp Outdated
Comment thread graph.cpp Outdated
Comment thread graph.cpp Outdated
Comment thread graph.cpp Outdated
@nickswalker

Copy link
Copy Markdown

At least

https://chris.beams.io/posts/git-commit/#imperative

but also probably fewer single line edit commits too

@nickswalker nickswalker left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why GitHub lets me make reviews on this but

Be careful not to track editor files. You can add these to the .gitignore if it helps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants