Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typo Fix #2490

Closed
wants to merge 9 commits into from
Closed

Typo Fix #2490

wants to merge 9 commits into from

Conversation

mmvergara
Copy link

@mmvergara mmvergara commented Jun 21, 2023

I got lazy sorry,

image

@realstealthninja
Copy link
Collaborator

Please do follow the pull request description given to you as it makes it easier for us to review

Copy link
Collaborator

@realstealthninja realstealthninja left a comment

Choose a reason for hiding this comment

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

Nice spotting the typo!

@Panquesito7 Panquesito7 added the bugfix Correction to existing algorithms label Jun 23, 2023
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Cool! Thanks. 🎉

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Looks like the whole file will need updating to our guidelines, as the CI is complaining about it.
Would you like to work on this, @mmvergara, or create an issue for the same?

@mmvergara
Copy link
Author

Looks like the whole file will need updating to our guidelines, as the CI is complaining about it. Would you like to work on this, @mmvergara, or create an issue for the same?

i'd like to work on it but sorry i don't really know how CI works, im kinda new to github. i might ruin something.

@realstealthninja
Copy link
Collaborator

Looks like the whole file will need updating to our guidelines, as the CI is complaining about it. Would you like to work on this, @mmvergara, or create an issue for the same?

i'd like to work on it but sorry i don't really know how CI works, im kinda new to github. i might ruin something.

no no he is asking you to fix the whole file and make it look better.
ie documenting code

@mmvergara
Copy link
Author

What did i just do lol, sorry i really don't know how the flow in these PR's work 💀

@realstealthninja
Copy link
Collaborator

you merged your pr with the main branch! i.e update your pull request so its up to date

Copy link
Collaborator

@realstealthninja realstealthninja left a comment

Choose a reason for hiding this comment

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

when we mean fix the algorithm we mean is document and comment on the algorithm
make it neat and understandable.
try reading the contribution guidelines

greedy_algorithms/dijkstra.cpp Outdated Show resolved Hide resolved
greedy_algorithms/dijkstra.cpp Outdated Show resolved Hide resolved
mmvergara and others added 5 commits June 26, 2023 13:16
Co-authored-by: realstealthninja <68815218+realstealthninja@users.noreply.github.com>
Co-authored-by: realstealthninja <68815218+realstealthninja@users.noreply.github.com>
@Panquesito7 Panquesito7 added automated tests are failing Do not merge until tests pass Improvement improvement in previously written codes and removed bugfix Correction to existing algorithms labels Jul 19, 2023
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

  1. Please use climits instead of limits.h.
  2. Do not use using namespace std. Use using <function> (such as std::cout) or directly their namespace name.
  3. Make sure to provide documentation and explanation of the algorithm, author, links, and self-test implementations.

If you need any help with this, let us know! 🙂

@mmvergara
Copy link
Author

  1. Please use climits instead of limits.h.
  2. Do not use using namespace std. Use using <function> (such as std::cout) or directly their namespace name.
  3. Make sure to provide documentation and explanation of the algorithm, author, links, and self-test implementations.

If you need any help with this, let us know! 🙂

aight yeah. can someone actually finish this PR for me, sorry i'm not so used to github or C++ or this algorithm (yet).

@Panquesito7
Copy link
Member

I'll work on this in the next few days. Thank you for contributing. 🙂

@Panquesito7 Panquesito7 self-assigned this Jul 21, 2023
Panquesito7 added a commit that referenced this pull request Jul 21, 2023
Originally initiated in #2490.
Co-authored-by: Mark Matthew Vergara <mmvergara@users.noreply.github.com>
Panquesito7 added a commit that referenced this pull request Jul 25, 2023
Originally initiated in #2490.
Co-authored-by: Mark Matthew Vergara <mmvergara@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automated tests are failing Do not merge until tests pass Improvement improvement in previously written codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants