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

Drop beta lint task all together #224

Closed
mulkieran opened this issue Sep 3, 2020 · 5 comments
Closed

Drop beta lint task all together #224

mulkieran opened this issue Sep 3, 2020 · 5 comments
Assignees

Comments

@mulkieran
Copy link
Member

mulkieran commented Sep 3, 2020

This is becoming less useful. It was introduced and the process that goes with it, under circumstances that were different in important ways from those that pertain now.

The good things about our current process:

  • We can observe and report lint regressions to clippy, which we have done at least once.
  • We address beta lints as they are generated by our PRs; so the person who's writing the code gets to do the fix. Probably they are the most knowledgeable. The cost is paid in small bits over six weeks instead of all at once at the end of the six weeks.

The bad thing in our current process:

  • Lots of PRs. We have to shift the beta task back to allowed failure every six weeks, then fix any new lints, then restore it. Pretty soon there will be four repos instead of three that we need to manage and there may be more later.
  • Trying to manage lints over multiple versions. This has proved a big pain recently w/ false positives in newly introduced lints that we have to use a [allow(clippy::all)] annotation to remove. And then, when the fix arrives, we have to remove that annotation. We got a suggestion that would improve things a bit, change the beta clippy lint task on Travis to allow the beta-only lint: Unable to allow same_item_push on beta and released Rust version rust-lang/rust-clippy#6002. We would still have to set and unset allows in the Travis task, but these would be clippy lints, not allows in source. This idea can be seen to work here: Deny all the pedantic clippy lints that we already obey stratisd#2201.
  • Having to check the beta lint on CI to see if it fails. I don't routinely run the lint task on beta in my development process and occasionally it fails.

The proposed change:

  • Drop the beta lint task altogether.

The new upgrade process would be:

  • fix all new lints (note that these new lints would be on stable, not beta)
  • give karma to new Fedora release
  • bump the recommended development toolchain
  • .... (the rest) would be unchanged
@mulkieran
Copy link
Member Author

@bgurney-rh I've assigned this to you...but we should discuss it in standup before we take any steps.

@mulkieran
Copy link
Member Author

We talked about this in standup. An alternative is to leave the lint task in as an allowed failure, for informational purposes. Developers would not address the beta lints in their PRs, so the lint failures that accumulated over the 6-week period would continue to accumulate throughout and be fixed only at the end of the cycle when the recommended development version bump is done.

@mulkieran
Copy link
Member Author

We talked about this in standup. An alternative is to leave the lint task in as an allowed failure, for informational purposes. Developers would not address the beta lints in their PRs, so the lint failures that accumulated over the 6-week period would continue to accumulate throughout and be fixed only at the end of the cycle when the recommended development version bump is done.

I'm not sure that this would be any more helpful than keeping a non-mandatory lint task on stable rather than beta. The sequence would typically be:

  • stable = recommended development (so linters agree)
  • new release, so stable > recommended development (new lints appear on stable branch)
    • these lints get fixed, then
  • recommended development bump, so stable = recommended development

@mulkieran
Copy link
Member Author

Ok. So the one action we want to take in the near future is to change the beta lint task into a stable lint task and make it always an allowed failure, not an intermittently allowed failure as it is now.

@mulkieran
Copy link
Member Author

Closed by attached PRs.

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

No branches or pull requests

2 participants