-
Notifications
You must be signed in to change notification settings - Fork 6
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
Push AVC validator to mod repos #113
base: master
Are you sure you want to change the base?
Conversation
73cdc38
to
ae224f1
Compare
Don't merge this till @DasSkelett is back from holiday break in January. |
Codewise it looks ok to me (I haven't had a thorough test/look over it yet), but I have concerns about bulk spraying PRs. I don't know that it's a good way to get buy in for its usage and feels a bit opinionated on our part. I am however in favour of making this as easy as possible for mod authors to add. It would be real nice to be able to trigger this from the frontend, say via a user logged into GitHub. But that's a much harder exercise. |
Do you think the core concept is salvageable? We could make the PR text more conciliatory, we could make the PR submission manual instead of automated (auto-generating the forks, branches, and commits would still be a huge help), we could limit to one PR per author per week, etc. I would really like to see broad adoption of this tool. |
Absolutely, I don't think the work is a waste at all. We could make it a scheduled task and set a flag on the mod in DynamnoDB. Somehow make it opt in, give us an opportunity to automate things like improvements to the process + raise new PRs. My only critique was that we want to be careful about our approach here, because It's definitely a GoodThing™. But we want buy in from the community, something that makes their lives easier, rather than CKAN having strong opinions about their workflow. |
ae224f1
to
468bd38
Compare
Rebased onto common argument framework from #122, I think it's right but worth a close look. |
The changes look to fit in with #122. I don't know that we've addressed the non-technical aspects of this PR or how we intend to use this. |
Motivation
@DasSkelett has written a cool GitHub Action to validate version files:
Currently deployment of this is opt-in, as mod authors find out about it. This may limit adoption.
Changes
Now a new
netkan avc-validator-pusher
command is created that:This way adoption of the validator can be as easy as clicking Merge. Hopefully this will get a large number of mod authors to sign on. As a bonus, it's a way of sending 200 PRs to LGG with one command.
Note that I want the PR body to link to this PR, so I'll be updating the code after it's created.
Name partially inspired by this song.