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

New branch #223

Closed
wants to merge 3 commits into from
Closed

New branch #223

wants to merge 3 commits into from

Conversation

swairik
Copy link
Contributor

@swairik swairik commented Oct 8, 2021

This PR implements the code for heap sort in c referred in issue #193

I have read the contributing guidelines in CONTRIBUTING.md and have ensured the following:

  • My code is placed in the required directory as outlined in CONTRIBUTING.md
  • My code file is named according to CONTRIBUTING.md
  • Variables, classes, functions etc. follow camel case as outlined in CONTRIBUTING.md
  • My code hasn't been implemented or claimed by someone else and I have checked the present issues and PRs for the same
  • My code is well commented
  • If my code is the first implementation of , I have added the link to this directory in README.md
  • I have not copied my code from anywhere

This PR creates/modifies the README for

I have read the contributing guidelines in CONTRIBUTING.md and have ensured the following:

  • Algo name (as heading)
  • A small description about algo
  • Time Complexity (Worst, Average, Best case)
  • Space Complexity (Worst, Average, Best case)
  • Logic and Pseudocode
  • Instructions for running code

Copy link
Collaborator

@codelixir codelixir left a comment

Choose a reason for hiding this comment

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

The PR description mentions only heap sort but the PR contains code for Kadane's algorithm as well.

Did you intend to make a single PR for two changes or is it by accident?

@swairik
Copy link
Contributor Author

swairik commented Oct 15, 2021

Yes, it was a mistake. I created heapsort and then I checked and saw that Kadane's Algo was not implemented in C. I intended on creating a separate branch but by mistake I committed them to same branch which got attached to the same PR.
I still see that Kadane's Algo has not been implemented in C in the project and would still like to contribute.
So is this PR acceptable or should I create another PR for Kadane's Algo?

@deutranium
Copy link
Owner

@swairik it's better if you can create different PRs per algo, would help in reviewing them separately!

@codelixir
Copy link
Collaborator

@swairik also I'd recommend you to give good names to pull requests, instead of something like "New branch"

@codelixir codelixir added the author response awaited The author has been notified of the changes but haven't responded label Oct 16, 2021
@swairik
Copy link
Contributor Author

swairik commented Oct 16, 2021

@codelixir can I then just close this pull request and create a new one following all the guidelines?

@codelixir
Copy link
Collaborator

Yes, that would be great, @swairik! Please open a new PR with just heap sort (I think Kadane has been accepted already), thanks.
Closing this one.

@codelixir codelixir closed this Oct 17, 2021
@swairik
Copy link
Contributor Author

swairik commented Oct 17, 2021

I have opened PR #232 implementing heap sort in C.
Apologies for the inconvenience caused (still a newbie here).
Thanks a lot for this amazing resource!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author response awaited The author has been notified of the changes but haven't responded
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants