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

Update named-literal-rules #1529

Merged
merged 2 commits into from
Feb 8, 2022
Merged

Update named-literal-rules #1529

merged 2 commits into from
Feb 8, 2022

Conversation

mlachkar
Copy link
Collaborator

fixes #1518

@mlachkar mlachkar requested a review from bjaglin January 25, 2022 16:06
@@ -86,7 +85,7 @@ output/src
```

Checkout the commit
[55f9196163ab0a](https://github.com/olafurpg/named-literal-arguments/commit/55f9196163ab0a5dde22364ab1f7880bf4a5dc54),
[55f9196163ab0a](https://github.com/scalacenter/scalafix-named-literal-arguments/commit/55f9196163ab0a5dde22364ab1f7880bf4a5dc54),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am wondering if we shouldn't rewrite the history to match the new APIs (instead of fixing it in a last commit together with the bump), WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have never done that.. why not! It's supposed to compile even when using old hash ! If we rewrite history, we need to rewrite more than one commit.. I don't know how many, but I guess it's doable. Do you think it's worth it ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that you have rewritten history, shouldn't this be scalacenter/scalafix-named-literal-arguments@12dce51?

All branches containing scalacenter/scalafix-named-literal-arguments@55f9196 should probably be updated (particularly the step-* ?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes it should!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It pushed to my clone repo instead of origin/mlachkar-patch-1.
I will update the step branches on scalafix-named-literals.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have updated the steps on the project

@@ -627,7 +626,7 @@ Another way to run custom rules from source is to use the `github:org/repo`
scheme.

```
scalafix --rules=github:olafurpg/named-literal-arguments
scalafix --rules=github:scalacenter/scalafix-named-literal-arguments
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you're right. It's not working at all.
it needs the directory scalafix + it's not the name of the repo, but the name of the rule: https://raw.githubusercontent.com/scalacenter/named-literal-arguments/master/rules/src/main/scala/fix/NamedLiteralArguments.scala

Do I rollback and put everything inside the directory scalafix ? It's annoying to have this directory sometimes, since ideas don't recognize it's scala code.

@mlachkar mlachkar requested a review from bjaglin February 3, 2022 17:59
@mlachkar
Copy link
Collaborator Author

mlachkar commented Feb 3, 2022

I have rewrite the history and re-create the hierarchy of folders!

Copy link
Collaborator

@bjaglin bjaglin left a comment

Choose a reason for hiding this comment

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

Thanks for rewriting the history on the other repo 👍 There are still some references to com.geirsson that you could update, otherwise LGTM!

docs/developers/tutorial.md Outdated Show resolved Hide resolved
Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
@mlachkar mlachkar merged commit 38583d9 into main Feb 8, 2022
@bjaglin bjaglin deleted the mlachkar-patch-1 branch February 9, 2022 23:20
bjaglin added a commit that referenced this pull request Feb 10, 2022
bjaglin added a commit to bjaglin/scalafix that referenced this pull request Feb 10, 2022
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.

Docs point to out-of-date / broken named-literal-arguments repo
2 participants