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

origin-manager: allow for new devel projects to be approved by review #2183

Closed
jberry-suse opened this issue Aug 29, 2019 · 6 comments · Fixed by #2283
Closed

origin-manager: allow for new devel projects to be approved by review #2183

jberry-suse opened this issue Aug 29, 2019 · 6 comments · Fixed by #2283
Assignees

Comments

@jberry-suse
Copy link
Contributor

Ideally this will also whitelist concurrent pending submissions from the same new devel project the same way the existing whitelist works utilized by factory-auto, but could end up just using the whitelist as a migration path.

Likely related, change_devel requests should simulate an origin_find() with the change in effect and trigger reviews based on that.

@jberry-suse
Copy link
Contributor Author

Had another case where this would be useful (much more common for Factory obviously).

@jberry-suse
Copy link
Contributor Author

Should also trigger the normal evaluation of policy after the devel project is specified (like adding additional reviews).

Another example: https://build.opensuse.org/request/show/731928

@lnussel
Copy link
Member

lnussel commented Sep 19, 2019

lack of reviews is critical IMO. prio needs to be raised. we miss review-team and legal for that.

@jberry-suse jberry-suse added P2 and removed P3 labels Sep 19, 2019
@jberry-suse
Copy link
Contributor Author

jberry-suse commented Oct 1, 2019

@lnussel What would you like the workflow to look like?

I see two main options:

  1. standard wait process when new package not found from any origin and require a comment command to process as devel (defaults to source project and package, but overridable via command)
  2. automatically fallback to acting as if source project and package are the devel (which will add necessary reviews at this point)

The first option would essentially trigger the second instead of it being automatic. If the second option an origin-reviewers review could be added to "approve coming from devel". Currently, no review is added for new packages coming from the highest available origin.

The benefit of second, is that currently no attention is drawn to waiting requests (except a comment), but that does not draw a release manager like a review could.

Related would be the Factory flow, I would expect it to always fallback (option 2) and add origin-reviewers to approve first time devel project. Presumably, Leap does not desire automatic approval for existing devel projects?

@coolo
Copy link
Member

coolo commented Oct 1, 2019

lack of reviews is critical IMO. prio needs to be raised. we miss review-team and legal for that.

BTW: licensedigger is reviewer in 15.2, so you don't miss it

@jberry-suse
Copy link
Contributor Author

jberry-suse commented Oct 25, 2019

I ran into a lot of issues:

  • test setup
    • pre-existing failures and assumptions that were intentionally hidden
    • OBS caching problems
    • odd mix of unnecessary custom data handling in harness that needs to be mixed with external script calls
    • tool cache layer assumptions broken by test workflow
  • origin-manager change_devel workflow
    • multi-policy handling for old and new origin overly complicated and not worth since majority case should be identical devel policies

As expected there were also a few small fixups exposed by the testing which makes it worth it. Currently, one of the main workflows covered with the second one mostly done, and the third should share a lot of code from other two. Also cover a variety of smaller workflows with tests. Hopefully now that major nuisances are resolved should be able to knock out remaining 1.5 worklows and have confidence old workflows remain function and new ones work.

Once everything is working a bit of polish will be required before submission.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment