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

Support passing rewriters to CLI #47

Merged
merged 8 commits into from Jun 12, 2020
Merged

Conversation

sl4vr
Copy link
Contributor

@sl4vr sl4vr commented Jun 8, 2020

What is the purpose of this pull request?

Support passing rewriters to CLI - #42

What changes did you make? (overview)

--list-rewriters and --rewrite options for nextify command.

Is there anything you'd like reviewers to focus on?

I didn't dig into that, just supposed rewriters gonna rewrite. Also it just failed for me in other case.

Also this implementation requires --edge and --proposed flags to use and even list corresponding rewriters. If it's not supposed to be so, I can change that.

Checklist

  • I've added tests for this change
  • I've added a Changelog entry
  • I've updated a documentation/SUPPORTED_FEATURES.md

@palkan
Copy link
Collaborator

palkan commented Jun 9, 2020

Thanks for the PR! I will take a look later this week.

@palkan palkan self-requested a review Jun 9, 2020
lib/ruby-next/commands/nextify.rb Outdated Show resolved Hide resolved
palkan
palkan approved these changes Jun 10, 2020
Copy link
Collaborator

@palkan palkan left a comment

Great job 👍 Thank you!

Could you please add a change log entry?

@sl4vr
Copy link
Contributor Author

sl4vr commented Jun 10, 2020

Thank you! I've just added change log entry.

@@ -2,6 +2,11 @@

## master

- Support passing rewriters to CLI. ([@sl4vr][])
Copy link
Collaborator

@palkan palkan Jun 12, 2020

Choose a reason for hiding this comment

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

👍
Could you please also add the actual link to the bottom of this file? (Right now it's not working, cause shortcut is not defined)

Copy link
Contributor Author

@sl4vr sl4vr Jun 12, 2020

Choose a reason for hiding this comment

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

Oops I thought it's Github feature or something 🙄

palkan
palkan approved these changes Jun 12, 2020
Copy link
Collaborator

@palkan palkan left a comment

Thanks a lot!
Awesome work 👍

Marked task as done: https://cultofmartians.com/tasks/ruby-next-cli-rewriters.html#task (check if everything is correct).

And could you please send me your mailing address to my e-mail (should be on my GitHub profile), so we'll we able to send you some stuff someday)

@palkan palkan merged commit 5eca048 into ruby-next:master Jun 12, 2020
20 checks passed
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.

None yet

3 participants