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

Tool to maintain repo and github metadata across all supported modules #8343

Closed
chillu opened this issue Aug 26, 2018 · 12 comments
Closed

Tool to maintain repo and github metadata across all supported modules #8343

chillu opened this issue Aug 26, 2018 · 12 comments

Comments

@chillu
Copy link
Member

chillu commented Aug 26, 2018

Create a script which syncs up standard metadata across our supported modules. We've gone past the point where this is feasible to achieve manually. See Robbie's suggestion in https://forum.silverstripe.org/t/improving-the-way-we-maintain-the-silverstripe-open-source-ecosystem/859.

Acceptance Criteria

  • Uses list of supported modules from the common JSON format
  • Can choose to run specific commands only (e.g. only update labels)
  • Runs in a few minutes (e.g. does sparse Github checkouts?)
  • Has an optional list where you can opt-out of specific modules for specific commands (e.g. those with a different license)
  • Uses local user for Github commits (assumes commit rights)

Script features

Notes

  • .travis.yml files are too complex for now
@robbieaverill
Copy link
Contributor

In terms of cloning repositories, it sounds like sparse checkouts would be useful for reading certain files across lots of repositories, but in order for us to sync back up to them I'm not sure it would help. Perhaps using shallow cloning with a depth of 1 would be better so we just fetch the head essentially, update the files then push back up again.

@robbieaverill
Copy link
Contributor

We could also synchronise the badges in readme files, although probably a low priority

@robbieaverill
Copy link
Contributor

I've made a start on this with a Symfony console application for syncing labels to all supported modules: https://github.com/creative-commoners/maintain. The logic is based on https://gist.github.com/chillu/45f9e9b777016f6257a64026aa84e0b3 and it will pull the modules list from https://raw.githubusercontent.com/silverstripe/supported-modules/gh-pages/modules.json. I'll change this to use https://addons.silverstripe.org/api/supported-addons at some point, although that's a composer package name and we really need the GitHub slug

@robbieaverill
Copy link
Contributor

Cross referencing:

@robbieaverill
Copy link
Contributor

I've made a start with the file sync command in silverstripe/cow#114. It'll currently pull the license from GitHub: https://github.com/silverstripe/supported-modules/blob/gh-pages/templates/LICENSE.md, then write, stage, commit and push any changes for each supported module.

We should add some extra metadata to https://github.com/silverstripe/supported-modules/blob/gh-pages/modules.json e.g. "sync_files": true for the modules we want this enabled for. Some supported modules we won't want this for, e.g. a LICENSE.md file with SilverStripe in the copyright wouldn't be right to push to a third party supported module, so we can skip it. For now I've just added a check to ensure it's under the SilverStripe GitHub account before pushing to it.

@dhensby
Copy link
Contributor

dhensby commented Sep 26, 2018

Ah, this is a good idea - it's been on my mind for a while. Things like branch protection set up and what not would be good too

@robbieaverill
Copy link
Contributor

Summarising some of the comments on the proposed update the PR template, and the previous thread around introducing a PR template:

We don't want to put people off contributing by making the process difficult

@dhensby: "I'm weary of making PRs more daunting and difficult to submit - are we actually addressing a problem that exists by implementing this template?"

The tone should be first person

@tractorcow: e.g. "I confirm the following" rather than "Please confirm the following"

Acknowledgement of code license

@dhensby: "If we are talking about giving up rights to the code in the PRs this probably needs to come as a more consistent and concerted effort across silverstripe projects and not just in framework and as a simple checkbox that can be edited after the fact." (@dhensby) - completing this task would achieve this

Make sure we're actually solving a problem by doing this

@dhensby (again): "I'm weary of making PRs more daunting and difficult to submit - are we actually addressing a problem that exists by implementing this template?"

Me: In general I don't think there are a large number of low quality PRs - the problem is that there are so many of them that aren't being reviewed or merged. I think that by introducing something like the proposed checklist at #8347 we can ease this a bit by giving reviewers more of a guarantee that PRs will conform to our contribution expectations before we even look at them.

Note @flamerohr said "The most important message is probably the "allow maintainers to make changes" permission to be set, so that maintainers could tailor parts accordingly if they have time - as opposed to wait 4 weeks for the creator to have time to get back to this."

^ I think that is enabled by default, but I agree. @dhensby agrees.

KISS

@jonom suggested having a single checkbox: "I've reviewed the contribution guidelines for issues and bugs."

@sminnee suggested a simple checklist, e.g. "have you run linting" etc

RFC: Add a CLA assistant

RFC: Add a CLA assistant - this hasn't gone through the RFC labelling process, but has five 👍reactions - four total core committers are in favour. Doing this could remove the need for PR templates entirely.

@chillu said on #7481: "I think the CLA tooling is a better way to solve this than checkboxes in PR templates"


New proposal

So with all of that combined, I think that implementing the CLA assistant (#7487) would be a good step, and either keep the PR template the way it is at the moment or remove it in favour of the CLA assistant.

Simple GitHub issue and PR templates can then be automatically synced to all commercially supported modules via a cow command.

@silverstripe/core-team are we happy with this?

@robbieaverill
Copy link
Contributor

Another thing that would be good to synchronise across modules is the phpcs ruleset. @unclecheese mentioned on #8569 that we should prefer single quotes over double quotes outside of required interpolation - this isn't part of PSR-2 but is a sensible rule we should enforce. Once all of our modules are relatively close to PSR-2 with a couple of exceptions like public function MyControllerMethod() and early returns in class_exists() checks we should be able to uniformly sync a ruleset to all modules. Framework is likely to be the most work here - see #7991

@robbieaverill
Copy link
Contributor

@chillu mentioned this new feature in GitHub this morning: https://help.github.com/en/articles/creating-a-default-community-health-file-for-your-organization

This would be good for CODE_OF_CONDUCT, CONTRIBUTING, and GitHub issue/PR template files. We wouldn't sync readmes and we don't really use SUPPORT files.

We'd need to remove existing files from repositories in order for this feature to work, since existing files take priority over organisation defaults. We could use this for placeholder readme files though.

We'd still need to use cow to sync licenses, phpunit/phpcs files, etc.

You cannot create a default license file for your organization. License files must be added to individual repositories so the file will be included when a project is cloned, packaged, or downloaded.

@dhensby
Copy link
Contributor

dhensby commented Mar 11, 2019

we don't really use SUPPORT files

We can start: https://github.com/silverstripe/silverstripe-framework/blob/4/SUPPORT.md

@robbieaverill
Copy link
Contributor

Touché! Nice =)

@GuySartorelli
Copy link
Member

This work has either been completed or is superceded by newer issues, respectively.

@GuySartorelli GuySartorelli closed this as not planned Won't fix, can't repro, duplicate, stale Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants