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

flutter/plugins pubspec presubmit needs tweaking #44749

Closed
mklim opened this issue Nov 12, 2019 · 6 comments
Closed

flutter/plugins pubspec presubmit needs tweaking #44749

mklim opened this issue Nov 12, 2019 · 6 comments
Labels
package flutter/packages repository. See also p: labels. team-infra Owned by Infrastructure team

Comments

@mklim
Copy link
Contributor

mklim commented Nov 12, 2019

Relatively recently we added a presubmit that attempts to verify that each PR increments the major, minor, or patch version in the pubspec.yaml.

It appears to be letting through false negatives in cases where the pubspec.yaml is never changed, but the CHANGELOG is updated: flutter/plugins#2242

This then cascades into later false positives, where future PRs are blocked for having incremented the pubspec.yaml version "too much": flutter/plugins#2264

Edit: @cyanglaz mentioned that the check deliberately ignores CHANGELOG, so that's what should probably be fixed here.

@mklim mklim added plugin team-infra Owned by Infrastructure team labels Nov 12, 2019
@godofredoc
Copy link
Contributor

Do you know if this is a test problem or a race condition in infra?

@mklim
Copy link
Contributor Author

mklim commented Nov 15, 2019

I haven't looked deeply into this but I think it's just an issue with the test itself. I'm pretty sure it's not catching all the changes that it should be linting for. I think it may only be checking to see if the pubspec.yaml file was changed and then failing then if the version upgrade wasn't right, when we should at least be checking for CHANGELOG too (and maybe any changed files at all).

Our entire infra has a race condition that can sort of effect his, where presubmits are run on the branch as they are and not the branch's changes applied to tip of tree master. So occasionally there are failures in post submit that "should" have gotten caught in presubmit, but the branch was too outdated to be tested in the failing state. But I don't think that's the main problem here.

@jmagman
Copy link
Member

jmagman commented Nov 19, 2019

Is this a duplicate of #15803?

@jmagman jmagman added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 19, 2019
@mklim
Copy link
Contributor Author

mklim commented Nov 19, 2019

I think this has a little more information about the current state of the world than that one, it was written before we had any checks at all. But having both seems redundant. I'll close the older in favor of this one.

@no-response no-response bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 19, 2019
@stuartmorgan
Copy link
Contributor

We're validating across the two files now, so closing as fixed.

@github-actions
Copy link

github-actions bot commented Aug 1, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021
@flutter-triage-bot flutter-triage-bot bot added the package flutter/packages repository. See also p: labels. label Jul 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package flutter/packages repository. See also p: labels. team-infra Owned by Infrastructure team
Projects
None yet
Development

No branches or pull requests

4 participants