[libvirt PATCH] github: enable lockdown of issues and merge requests
Libvirt uses GitHub as an automated read-only mirror. The goals were to have a disaster recovery backup for libvirt.org, a way to make it easy for people to clone their own private copy of libvirt Git, and finally as a way to interact with apps like Travis. The project description was set to a message telling people that we don't respond to pull requests. This was quite a negative message to potential contributors, and also did not give them any guidance about the right way to submit to libvirt. Many also missed the description and submitted issues or pull requests regardless. It is possible to disable the issue tracker in GitHub, but there is no way to disable merge requests. Disabling the issue tracker would also leave the problem of users not being given any positive information about where they should be reporting instead. There is a fairly new 3rd party application built for GitHub that provides a bot which auto-responds to both issues and merge requests, closing and locking them, with a arbitrary comment: https://github.com/apps/repo-lockdown This commit adds a suitable configuration file for libvirt, which tries to give a positive response to user's issue/pullreq and guide them to the desired contribution path on GitLab. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- FYI, I have configured the test.git repo to demonstrate the operation. See the comments / actions performed here: https://github.com/libvirt/test/pull/3 https://github.com/libvirt/test/issues/2 This lockdown.yml file will need to be created in each libvirt git repo, with just the URL changed. If this patch is approved, then I propose to directly push the equivalent config file to every other git repo, without spamming libvir-list for review of each. Ideally we should be creating a CONTRIBUTING.md file in each repo too, as both GitHub and GitLab direct users to look at this file for information about how to contribute. This is how we could guide people to use GitLab in the first place, avoiding them filing issues/pulls on the wrong site and triggering this bot. .github/lockdown.yml | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 .github/lockdown.yml diff --git a/.github/lockdown.yml b/.github/lockdown.yml new file mode 100644 index 0000000000..09cbdf1b4a --- /dev/null +++ b/.github/lockdown.yml @@ -0,0 +1,36 @@ +# Configuration for Repo Lockdown - https://github.com/dessant/repo-lockdown + +# Close issues and pull requests +close: true + +# Lock issues and pull requests +lock: true + +# Optionally, specify configuration settings just for `issues` or `pulls` +issues: + comment: | + Thank you for your interest in the libvirt project. + + Since this repository is a read-only mirror of the project's master repostory hosted on GitLab, issues opened here are not processed. + + We kindly request that new issues are reported to + + https://gitlab.com/libvirt/libvirt/-/issues/new + + Thank you for your time and understanding. + +pulls: + comment: | + Thank you for your interest in the libvirt project. + + Since this repository is a read-only mirror of the project's master repostory hosted on GitLab, merge requests opened here are not processed. + + We kindly request that contributors fork the project at + + https://gitlab.com/libvirt/libvirt/ + + push changes to the fork, and then open a new merge request at + + https://gitlab.com/libvirt/libvirt/-/merge_requests/new + + Thank you for your time and understanding. -- 2.24.1
On 4/3/20 5:38 AM, Daniel P. Berrangé wrote:
Libvirt uses GitHub as an automated read-only mirror. The goals were to have a disaster recovery backup for libvirt.org, a way to make it easy for people to clone their own private copy of libvirt Git, and finally as a way to interact with apps like Travis.
The project description was set to a message telling people that we don't respond to pull requests. This was quite a negative message to potential contributors, and also did not give them any guidance about the right way to submit to libvirt. Many also missed the description and submitted issues or pull requests regardless.
It is possible to disable the issue tracker in GitHub, but there is no way to disable merge requests. Disabling the issue tracker would also leave the problem of users not being given any positive information about where they should be reporting instead.
There is a fairly new 3rd party application built for GitHub that provides a bot which auto-responds to both issues and merge requests, closing and locking them, with a arbitrary comment:
https://github.com/apps/repo-lockdown
This commit adds a suitable configuration file for libvirt, which tries to give a positive response to user's issue/pullreq and guide them to the desired contribution path on GitLab.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> ---
FYI, I have configured the test.git repo to demonstrate the operation.
See the comments / actions performed here:
https://github.com/libvirt/test/pull/3 https://github.com/libvirt/test/issues/2
This lockdown.yml file will need to be created in each libvirt git repo, with just the URL changed. If this patch is approved, then I propose to directly push the equivalent config file to every other git repo, without spamming libvir-list for review of each.
Makes sense to me.
+++ b/.github/lockdown.yml @@ -0,0 +1,36 @@ +# Configuration for Repo Lockdown - https://github.com/dessant/repo-lockdown + +# Close issues and pull requests +close: true + +# Lock issues and pull requests +lock: true + +# Optionally, specify configuration settings just for `issues` or `pulls` +issues: + comment: | + Thank you for your interest in the libvirt project. + + Since this repository is a read-only mirror of the project's master repostory hosted on GitLab, issues opened here are not processed.
Is line-wrapping this worthwhile? Otherwise, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Fri, Apr 03, 2020 at 08:55:11AM -0500, Eric Blake wrote:
On 4/3/20 5:38 AM, Daniel P. Berrangé wrote:
Libvirt uses GitHub as an automated read-only mirror. The goals were to have a disaster recovery backup for libvirt.org, a way to make it easy for people to clone their own private copy of libvirt Git, and finally as a way to interact with apps like Travis.
The project description was set to a message telling people that we don't respond to pull requests. This was quite a negative message to potential contributors, and also did not give them any guidance about the right way to submit to libvirt. Many also missed the description and submitted issues or pull requests regardless.
It is possible to disable the issue tracker in GitHub, but there is no way to disable merge requests. Disabling the issue tracker would also leave the problem of users not being given any positive information about where they should be reporting instead.
There is a fairly new 3rd party application built for GitHub that provides a bot which auto-responds to both issues and merge requests, closing and locking them, with a arbitrary comment:
https://github.com/apps/repo-lockdown
This commit adds a suitable configuration file for libvirt, which tries to give a positive response to user's issue/pullreq and guide them to the desired contribution path on GitLab.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> ---
FYI, I have configured the test.git repo to demonstrate the operation.
See the comments / actions performed here:
https://github.com/libvirt/test/pull/3 https://github.com/libvirt/test/issues/2
This lockdown.yml file will need to be created in each libvirt git repo, with just the URL changed. If this patch is approved, then I propose to directly push the equivalent config file to every other git repo, without spamming libvir-list for review of each.
Makes sense to me.
+++ b/.github/lockdown.yml @@ -0,0 +1,36 @@ +# Configuration for Repo Lockdown - https://github.com/dessant/repo-lockdown + +# Close issues and pull requests +close: true + +# Lock issues and pull requests +lock: true + +# Optionally, specify configuration settings just for `issues` or `pulls` +issues: + comment: | + Thank you for your interest in the libvirt project. + + Since this repository is a read-only mirror of the project's master repostory hosted on GitLab, issues opened here are not processed.
Is line-wrapping this worthwhile?
The content here is treated as literal markdown, with line breaks preserved. If I line wrap it here, then it gets line wrapped when displayed to the user which is not desirable - I want to let the browser wrap according to the space it has to display the comment. Using ">" instead of "|" for the multiple line string lets you do wrapping, which is then ignored, but this has the annoying side effect of also removing the blank lines between paragraphs. So the long lines were the least worst option to give the desired display in the browser that I came up with.
Otherwise, Reviewed-by: Eric Blake <eblake@redhat.com>
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, 2020-04-03 at 11:38 +0100, Daniel P. Berrangé wrote:
+++ b/.github/lockdown.yml @@ -0,0 +1,36 @@ +pulls: + comment: | + Thank you for your interest in the libvirt project. + + Since this repository is a read-only mirror of the project's master repostory hosted on GitLab, merge requests opened here are not processed.
Maybe s/merge/pull/ here? Since we're adding a comment on GitHub and targeting GitHub users when doing so, it might be more appropriate to use the GitHub term. Everything else looks good, but there's the obvious caveat that we can't merge the commit as-is until we have actually moved to GitLab. Since that could take a while, and locking down the GitHub repositories is already a good idea, maybe point people to https://libvirt.org/hacking.html in the meantime? -- Andrea Bolognani / Red Hat / Virtualization
On a Friday in 2020, Andrea Bolognani wrote:
On Fri, 2020-04-03 at 11:38 +0100, Daniel P. Berrangé wrote:
+++ b/.github/lockdown.yml @@ -0,0 +1,36 @@ +pulls: + comment: | + Thank you for your interest in the libvirt project. + + Since this repository is a read-only mirror of the project's master repostory hosted on GitLab, merge requests opened here are not processed.
Maybe s/merge/pull/ here? Since we're adding a comment on GitHub and targeting GitHub users when doing so, it might be more appropriate to use the GitHub term.
Everything else looks good, but there's the obvious caveat that we can't merge the commit as-is until we have actually moved to GitLab.
Since that could take a while, and locking down the GitHub repositories is already a good idea, maybe point people to
https://libvirt.org/hacking.html
in the meantime?
As danpb mentions in the cover letter, CONTRIBUTING.md should be easily discoverable on GitLab. We [0] should somehow put the brief instructions there (like README-hacking) and not scare drive-by contributors with the giant hacking.html. OTOH, I'm not sure how well 'I fixed these coding styles issues and pushed the patch' translates to the merge request review. Jano [0] it's on my TODO list (TM)
-- Andrea Bolognani / Red Hat / Virtualization
On Fri, 2020-04-03 at 19:35 +0200, Ján Tomko wrote:
On a Friday in 2020, Andrea Bolognani wrote:
Everything else looks good, but there's the obvious caveat that we can't merge the commit as-is until we have actually moved to GitLab.
Since that could take a while, and locking down the GitHub repositories is already a good idea, maybe point people to
https://libvirt.org/hacking.html
in the meantime?
As danpb mentions in the cover letter, CONTRIBUTING.md should be easily discoverable on GitLab. We [0] should somehow put the brief instructions there (like README-hacking) and not scare drive-by contributors with the giant hacking.html.
The problem with hacking.html is that it's a catch-all location that where a hodgepodge of concepts, which are related to various degrees, are documented: * various resources related to libvirt development, such as the primary git repository and the libvirt project on Zanata; * how to write good commit messages; * how to split changes into multiple commits; * other requirements, such as having DCO information; * how to run the test suite; * how to fix issues reported by valgrind; * how to send patches to libvir-list; * which languages are appropriate for new code; * what's our code style; * which GLib APIs should be used instead of libvirt-specific APIs that in most cases have been dropped already; * governance rules; all of it sprinkled with useful tips such as which options to use when running 'git diff'. No wonder it's overwhelming. I think we should rethink how we organize this information. For example, while the fact that git-publish should be used to post patches is, at least until we complete the move to GitLab, critical information to new contributors, but detailed steps for setting up mail delivery in git could live in a separate guide linked to from the main document, so that only those who are not already familiar with a mail-based development workflow need to access it; along the same line, how to run the test suite should be documented prominently but how to solve issues reported by valgrind doesn't. Of course the biggest offender is the coding style, which takes up most of the page. That could be split off to a separate page too, under the IMHO sane assumption that even someone unfamiliar with the codebase will naturally get very close to adhering to it through a combination of looking at existing code and addressing the issues reported by 'make syntax-check'; in the longer run, we should really invest some time building a clang-format profile that approximate our current coding style and just require that all contributions go through that tool, thus making the page mostly obsolete. Anyway, back to CONTRIBUTING.md specifically: once we have improved and trimmed down hacking.html (contributing.html?) to a reasonable size, we can simply link to it. I don't think that's a strict dependency, however, and while our current hacking.html is clearly suboptimal I'd rather have issues/PRs locked down and directing developers to it than the current status quo.
OTOH, I'm not sure how well 'I fixed these coding styles issues and pushed the patch' translates to the merge request review.
I don't think it does. However, once syntax-check is something that is run automatically at MR time and which gates merge, contributors are more likely to address the issue themselves before a reviewer gets around to looking at the changes. -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Apr 06, 2020 at 10:34:53AM +0200, Andrea Bolognani wrote:
On Fri, 2020-04-03 at 19:35 +0200, Ján Tomko wrote:
On a Friday in 2020, Andrea Bolognani wrote:
Everything else looks good, but there's the obvious caveat that we can't merge the commit as-is until we have actually moved to GitLab.
Since that could take a while, and locking down the GitHub repositories is already a good idea, maybe point people to
https://libvirt.org/hacking.html
in the meantime?
As danpb mentions in the cover letter, CONTRIBUTING.md should be easily discoverable on GitLab. We [0] should somehow put the brief instructions there (like README-hacking) and not scare drive-by contributors with the giant hacking.html.
The problem with hacking.html is that it's a catch-all location that where a hodgepodge of concepts, which are related to various degrees, are documented:
* various resources related to libvirt development, such as the primary git repository and the libvirt project on Zanata; * how to write good commit messages; * how to split changes into multiple commits; * other requirements, such as having DCO information; * how to run the test suite; * how to fix issues reported by valgrind; * how to send patches to libvir-list; * which languages are appropriate for new code; * what's our code style; * which GLib APIs should be used instead of libvirt-specific APIs that in most cases have been dropped already; * governance rules;
all of it sprinkled with useful tips such as which options to use when running 'git diff'. No wonder it's overwhelming.
I think we should rethink how we organize this information. For example, while the fact that git-publish should be used to post patches is, at least until we complete the move to GitLab, critical information to new contributors, but detailed steps for setting up mail delivery in git could live in a separate guide linked to from the main document, so that only those who are not already familiar with a mail-based development workflow need to access it; along the same line, how to run the test suite should be documented prominently but how to solve issues reported by valgrind doesn't.
Of course the biggest offender is the coding style, which takes up most of the page. That could be split off to a separate page too, under the IMHO sane assumption that even someone unfamiliar with the codebase will naturally get very close to adhering to it through a combination of looking at existing code and addressing the issues reported by 'make syntax-check'; in the longer run, we should really invest some time building a clang-format profile that approximate our current coding style and just require that all contributions go through that tool, thus making the page mostly obsolete.
Anyway, back to CONTRIBUTING.md specifically: once we have improved and trimmed down hacking.html (contributing.html?) to a reasonable size, we can simply link to it. I don't think that's a strict dependency, however, and while our current hacking.html is clearly suboptimal I'd rather have issues/PRs locked down and directing developers to it than the current status quo.
IMHO the CONTRIBUTING.md is something that can be fairly simple. The important things is that it should cover directly are - How to submit patches - How to report issues - How to comply with the DCO Then it should provide links to information about coding style, information around testing, information about platform portability expectations, and other important reference material that might be relevant. We can certainly split up / re-organize hacking.html more generally, but I don't think that's a strict pre-requisite for adding a simple CONTRIBUTING.md file. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, 2020-04-06 at 10:02 +0100, Daniel P. Berrangé wrote:
On Mon, Apr 06, 2020 at 10:34:53AM +0200, Andrea Bolognani wrote:
Anyway, back to CONTRIBUTING.md specifically: once we have improved and trimmed down hacking.html (contributing.html?) to a reasonable size, we can simply link to it. I don't think that's a strict dependency, however, and while our current hacking.html is clearly suboptimal I'd rather have issues/PRs locked down and directing developers to it than the current status quo.
IMHO the CONTRIBUTING.md is something that can be fairly simple. The important things is that it should cover directly are
- How to submit patches - How to report issues - How to comply with the DCO
Then it should provide links to information about coding style, information around testing, information about platform portability expectations, and other important reference material that might be relevant.
We can certainly split up / re-organize hacking.html more generally,
I'll try to give this a shot.
but I don't think that's a strict pre-requisite for adding a simple CONTRIBUTING.md file.
I'm wary of the risk of duplicating information in multiple places. We already have some overlapping information between hacking.html and README-hacking, but that's justified by the fact that it was considered a good idea to provide "bootstrap" steps, ie. enough information to get to the point where you could build the full HTML documentation locally from a git clone. I'm no longer convinced this is that useful, because if you have a git clone you clearly have internet access and at that point you might as well jump straight to the pre-built version available on libvirt.org, but let's leave this argument for another day. What we do *not* want to have is another "quick start guide" that duplicates the information further. If our hacking.html is too overwhelming to serve as such a landing page, then we should change it so that it no longer is, not add yet another overlapping document. -- Andrea Bolognani / Red Hat / Virtualization
On Fri, Apr 03, 2020 at 07:23:03PM +0200, Andrea Bolognani wrote:
On Fri, 2020-04-03 at 11:38 +0100, Daniel P. Berrangé wrote:
+++ b/.github/lockdown.yml @@ -0,0 +1,36 @@ +pulls: + comment: | + Thank you for your interest in the libvirt project. + + Since this repository is a read-only mirror of the project's master repostory hosted on GitLab, merge requests opened here are not processed.
Maybe s/merge/pull/ here? Since we're adding a comment on GitHub and targeting GitHub users when doing so, it might be more appropriate to use the GitHub term.
Everything else looks good, but there's the obvious caveat that we can't merge the commit as-is until we have actually moved to GitLab.
Now that we've got the release out of the way, I want to do the switch to GitLab as the master repository in the next day or two. I've got everything prepared that we need now I believe. Must just flip the repo mirror job into reverse and make the old repos read-only to current maintainers to prevent acceidental pushes to the wrong place. I'll plan to do this on Tuesday, approx 1200 GMT give/take an hour. Maintainers will merely need to update their git origin URL, nothing more. Although this lockdown message suggests opening a merge request, and we wouldn't immediately be using merge requests for libvirt.git, I dont think that's a problem. The number of people who'll see and follow this lockdown message is going to be pretty small to start with at least, so its not a big deal if we tell them to use MR right now. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, 2020-04-06 at 09:57 +0100, Daniel P. Berrangé wrote:
Although this lockdown message suggests opening a merge request, and we wouldn't immediately be using merge requests for libvirt.git, I dont think that's a problem. The number of people who'll see and follow this lockdown message is going to be pretty small to start with at least, so its not a big deal if we tell them to use MR right now.
That's fair, but even for those few people why wouldn't we point them to the current documentation for the time being and only start pointing them to GitLab merge requests once we actually move to that workflow? -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Apr 06, 2020 at 11:18:51AM +0200, Andrea Bolognani wrote:
On Mon, 2020-04-06 at 09:57 +0100, Daniel P. Berrangé wrote:
Although this lockdown message suggests opening a merge request, and we wouldn't immediately be using merge requests for libvirt.git, I dont think that's a problem. The number of people who'll see and follow this lockdown message is going to be pretty small to start with at least, so its not a big deal if we tell them to use MR right now.
That's fair, but even for those few people why wouldn't we point them to the current documentation for the time being and only start pointing them to GitLab merge requests once we actually move to that workflow?
I think that's just unnecessary churn, with no real benefit, especially since this lockdown will be replicated across all the repos. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, 2020-04-06 at 10:21 +0100, Daniel P. Berrangé wrote:
On Mon, Apr 06, 2020 at 11:18:51AM +0200, Andrea Bolognani wrote:
On Mon, 2020-04-06 at 09:57 +0100, Daniel P. Berrangé wrote:
Although this lockdown message suggests opening a merge request, and we wouldn't immediately be using merge requests for libvirt.git, I dont think that's a problem. The number of people who'll see and follow this lockdown message is going to be pretty small to start with at least, so its not a big deal if we tell them to use MR right now.
That's fair, but even for those few people why wouldn't we point them to the current documentation for the time being and only start pointing them to GitLab merge requests once we actually move to that workflow?
I think that's just unnecessary churn, with no real benefit, especially since this lockdown will be replicated across all the repos.
I disagree, but on the other hand I *do* agree that this is likely to only hit a very small population, so I won't stand in the way. Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Apr 06, 2020 at 09:57:46AM +0100, Daniel P. Berrangé wrote:
On Fri, Apr 03, 2020 at 07:23:03PM +0200, Andrea Bolognani wrote:
On Fri, 2020-04-03 at 11:38 +0100, Daniel P. Berrangé wrote:
+++ b/.github/lockdown.yml @@ -0,0 +1,36 @@ +pulls: + comment: | + Thank you for your interest in the libvirt project. + + Since this repository is a read-only mirror of the project's master repostory hosted on GitLab, merge requests opened here are not processed.
Maybe s/merge/pull/ here? Since we're adding a comment on GitHub and targeting GitHub users when doing so, it might be more appropriate to use the GitHub term.
Everything else looks good, but there's the obvious caveat that we can't merge the commit as-is until we have actually moved to GitLab.
Now that we've got the release out of the way, I want to do the switch to GitLab as the master repository in the next day or two. I've got everything prepared that we need now I believe. Must just flip the repo mirror job into reverse and make the old repos read-only to current maintainers to prevent acceidental pushes to the wrong place. I'll plan to do this on Tuesday, approx 1200 GMT give/take an hour. Maintainers will merely need to update their git origin URL, nothing more.
Although this lockdown message suggests opening a merge request, and we wouldn't immediately be using merge requests for libvirt.git, I dont think that's a problem. The number of people who'll see and follow this lockdown message is going to be pretty small to start with at least, so its not a big deal if we tell them to use MR right now.
Sounds good to me. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (5)
-
Andrea Bolognani -
Daniel P. Berrangé -
Eric Blake -
Ján Tomko -
Pavel Hrdina