Skip to content

K Closest Points to Origin #77

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

K Closest Points to Origin #77

wants to merge 10 commits into from

Conversation

Chaitra-Bhat383
Copy link
Contributor

No description provided.

ignacio-chiazzo
ignacio-chiazzo reviewed Oct 23, 2022
View reviewed changes
Copy link
Owner

@ignacio-chiazzo ignacio-chiazzo left a comment

Choose a reason for hiding this comment

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

The problem/solution looks good to me. Could you fix the linter issue by running npx eslint LeetcodeProblems LeetcodeProblemsTests --fix? thanks!

@ignacio-chiazzo
Copy link
Owner

I just enabled the linter to run on this PR. You can also check lint errors locally by running npx eslint LeetcodeProblems LeetcodeProblemsTests

@Chaitra-Bhat383
Copy link
Contributor Author

I resolved all the linter errors, please have a check!

ignacio-chiazzo
ignacio-chiazzo reviewed Oct 23, 2022
View reviewed changes
Copy link
Owner

@ignacio-chiazzo ignacio-chiazzo left a comment

Choose a reason for hiding this comment

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

I see some linting errors in LeetcodeProblems/Algorithms/K_Closest_Points_to_Origin.js file
Screen Shot 2022-10-23 at 10 45 32 AM

README.md Outdated
3) An entry of the problem in the list of solutions within the README file.

PR Example: https://github.com/ignacio-chiazzo/Algorithms-Leetcode-Javascript/pull/39
[Maximal Square ](/LeetcodeProblems/Maximal_Square.js) | Medium | https://leetcode.com/problems/maximal-square/
Copy link
Owner

Choose a reason for hiding this comment

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

Could you update the readme? It deleted all the previous staff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes did it now!

ignacio-chiazzo
ignacio-chiazzo reviewed Oct 23, 2022
View reviewed changes
Copy link
Owner

@ignacio-chiazzo ignacio-chiazzo left a comment

Choose a reason for hiding this comment

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

oh I see what's going on. Could you rebase? A bad commit has been reverted and it caused to have two readmes. #78

@Chaitra-Bhat383
Copy link
Contributor Author

I hope its right now!

ignacio-chiazzo
ignacio-chiazzo reviewed Oct 23, 2022
View reviewed changes
Copy link
Owner

@ignacio-chiazzo ignacio-chiazzo left a comment

Choose a reason for hiding this comment

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

It looks good now, Could you rebase with the latest master so that I can hit rebase and merge and add you as a contributor?

@Chaitra-Bhat383
Copy link
Contributor Author

Could you merge it now?

@ignacio-chiazzo
Copy link
Owner

ignacio-chiazzo commented Oct 23, 2022 •
edited
Loading

Could you rebase with the latest master? An easy way to do it would be to squash all your 10 commits into 1 and then rebase with the latest master. I can do it for you but GitHub won't mark you as a contributor if I'm not mistaken.

Screen Shot 2022-10-23 at 3 43 35 PM

@Chaitra-Bhat383 Chaitra-Bhat383 closed this by deleting the head repository Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ignacio-chiazzo ignacio-chiazzo ignacio-chiazzo left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Chaitra-Bhat383 @ignacio-chiazzo