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

The great rename #326

Merged
merged 16 commits into from
Sep 6, 2017
Merged

The great rename #326

merged 16 commits into from
Sep 6, 2017

Conversation

olafurpg
Copy link
Contributor

@olafurpg olafurpg commented Sep 6, 2017

  • s/Rewrite/Rule/ A mostly backwards source and binary compatible API will remain throughout v0.5. Fixes Rename Rewrite to Checker/Rule? #293
  • s/ExplicitReturnTypes/ExplicitResultTypes/
  • no more implicit macro magic to infer rule names, they must be provided explicitly from now on.

This setup is more verbose but it's less magic.
This is more appropriate scala lingo, according to Seb.
This scheme involves less boilerplate for rule authors.
Turns out the tests from NoValInForComprehension were still using the
old .source testing system.
@olafurpg olafurpg requested a review from gabro September 6, 2017 14:04
CONTRIBUTING.md Outdated
@@ -3,7 +3,7 @@ Contributing

## Modules

- `scalafix-core/` the rewrite rules
- `scalafix-core/` data structures and algorithms for scalafix rules (linters/checkers/rewrites/fixes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

linters/checkers/rewrites/fixes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed!

you can load it with the @code{https:} or @code{http:} protocol
@config
rewrite = "https://gist.githubusercontent.com/olafurpg/fc6f43a695ac996bd02000f45ed02e63/raw/dfce7784d61dffda183a582b9a1750399c2c465b/ExampleRewrite.scala"
rule = "https://gist.githubusercontent.com/olafurpg/fc6f43a695ac996bd02000f45ed02e63/raw/f96587ec5afe8ed139b2c543c65a417dcafca79e/ExampleRewrite.scala"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you rename the file on gist to ExampleRule?

@config
explicitReturnTypes.memberKind = [Val, Def, Var]
explicitReturnTypes.memberVisibility = [Public, Protected]
// Experimental, shorten fully qualified names and insert missing imports
// By default, names are fully qualified and prefixed with _root_.
unsafeShortenNames = true // false by default.

@sect{ExplicitReturnTypes}
This rewrite has been renamed into @sect.ref{ExplicitResultTypes}
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/rewrite/rule

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Old habit, wrote it after running sed replace. Fixed.

import metaconfig.ConfDecoder
import metaconfig.Configured

/** A Scalafix Rule is a checker/linter/rewrite/auto-fixer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above, this checker/linter/rewrites/whatever is a bit confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, removed.


/** A Scalafix Rule is a checker/linter/rewrite/auto-fixer.
*
* To implement a rewrite/auto-fixer, override the `fix` method. Example:
Copy link
Collaborator

@gabro gabro Sep 6, 2017

Choose a reason for hiding this comment

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

Maybe

To provide an automatic fix for this rule, ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, done.

* }
* }}}
*
* To implement a linter/checker, override the `check` method. Example:
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above, I would pick a name and stick to it instead of providing alternatives linter/checker

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, went with "To report violations of this rule (without automatic fix)"

Copy link
Contributor Author

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Thank you for the review @gabro !

CONTRIBUTING.md Outdated
@@ -3,7 +3,7 @@ Contributing

## Modules

- `scalafix-core/` the rewrite rules
- `scalafix-core/` data structures and algorithms for scalafix rules (linters/checkers/rewrites/fixes)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed!

@config
explicitReturnTypes.memberKind = [Val, Def, Var]
explicitReturnTypes.memberVisibility = [Public, Protected]
// Experimental, shorten fully qualified names and insert missing imports
// By default, names are fully qualified and prefixed with _root_.
unsafeShortenNames = true // false by default.

@sect{ExplicitReturnTypes}
This rewrite has been renamed into @sect.ref{ExplicitResultTypes}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Old habit, wrote it after running sed replace. Fixed.

import metaconfig.ConfDecoder
import metaconfig.Configured

/** A Scalafix Rule is a checker/linter/rewrite/auto-fixer.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, removed.


/** A Scalafix Rule is a checker/linter/rewrite/auto-fixer.
*
* To implement a rewrite/auto-fixer, override the `fix` method. Example:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, done.

* }
* }}}
*
* To implement a linter/checker, override the `check` method. Example:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, went with "To report violations of this rule (without automatic fix)"

@olafurpg olafurpg merged commit f62e68d into scalacenter:master Sep 6, 2017
@olafurpg
Copy link
Contributor Author

olafurpg commented Sep 6, 2017

:shipit: 🎆

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.

2 participants