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

Add rewrite-remote installation support #121

Merged
merged 5 commits into from
Oct 2, 2024
Merged

Conversation

OlegDokuka
Copy link
Contributor

What's changed?

What's your motivation?

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you have non-standard settings for the imports. The import order is different and there are no star-imports when there are at least 5 imports from a package. Please reset your Java formatter settings in IDEA to the defaults.

"version": "1.0.0",
"description": "",
"dependencies": {
"@openrewrite/rewrite-remote": "^0.3.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about it, it might be a good idea to use ~ instead. That would allow us to still roll out some fixes to the parser, which don't require changes the LST model or printer. At the same time it would allow us to create a new 0.4.0 release for whenever we want to merge changes to the LST model without causing the CLI to break. This is what I did for Python.

What do you think? The only issue I see with this is that we have to remember to update this. But since we currently have to update the package.json version manually anyway, it's maybe OK. Also, we can automate some of that.

Suggested change
"@openrewrite/rewrite-remote": "^0.3.0"
"@openrewrite/rewrite-remote": "~0.3.0"

Copy link
Contributor

@knutwannheden knutwannheden Oct 1, 2024

Choose a reason for hiding this comment

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

Now I think I also remember that I did some automation here for Python, so that it at build time reads the current version of rewrite-remote and writes that to the bundled requirements.txt file. That way we don't have to worry about forgetting to update it. We could do the same thing here.

https://github.com/openrewrite/rewrite-python/blob/a01e9b5938ce6f2539c064f84eab4b6126500400/build.gradle.kts#L56-L64

@OlegDokuka OlegDokuka merged commit 20e859d into main Oct 2, 2024
2 checks passed
@OlegDokuka OlegDokuka deleted the main-installable-parser branch October 2, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants