Accidental issue closures while setting up package website

[olexandr-konovalov]

I was publishing new release of Wedderga and was surprised to see that issue https://github.com/gap-packages/wedderga/issues/8 has been closed by me in this commit.

It was actually this commit in GitHubPagesForGAP, but when I've pushed it to my gh-pages branch while setting up package's new website, Resolves #8 in the commit message has been interpreted as referring to issue #8 in Wedderga.

This may at least surprise package authors, and may accidentally put important issues and PRs below the radar. Is there some command line option to disable automated closure while pulling from GitHubPagesForGAP? Or should the history of GitHubPagesForGAP be rewritten to remove words triggering closures?

[fingolfin]

That's odd. According to https://help.github.com/articles/closing-issues-using-keywords/, "When the pull request is merged into your repository's default branch, the corresponding issue is automatically closed." Since gh-pages is not your default branch, this should not happen -- well, OK, from a mathematical point of view, they did not say anything about what happens to branches which are not the default branch, but I think the implied meaning is clear.

I'll contact GitHub support to ask whether this is a bug in the documentation or in the site's behavior.

Rewriting the history of any project is something I'd very reluctant to do. For GitHubPagesForGAP, it'd be particularly bad, as that would kill the upgrade path - I'd the have to come up with a complicate extra set of instructions in order to allow package maintainers using it to sidegrade to the rewriting history. Not something I'd like to do.

[olexandr-konovalov]

@fingolfin thanks - also note that there was no pull request here, just push. Let's see what they will reply.

Yeah, that is convincing point against rewriting history. I set up recently websites for a bunch of my packages migrated to GitHub, and since none of them had as many issues as 8, this did not happen there. But as time goes by, they may get more numbers used, and next upgrade of GitHubPagesForGAP may trigger this again...

[fingolfin]

I will take care not to put "closes #XYZ" comments in future commit messages for that repos, so as a matter of fact, your next upgrade of GitHubPagesForGAP will not trigger this again.

[fingolfin]

There is nothing else I can do about this, I am afraid, short of rewriting the history of this project -- which would then cause all packages using GitHubPagesForGAP major pain if they ever want to upgrade to the latest version.

[olexandr-konovalov]

Ok, as soon as everyone contributing to this repo and reviewing commits remembers to not to put "closes #XYZ" comments in future commit messages.

[fingolfin]

Well, yeah, sure. Just like everybody in general should remember not to let bugs slip through in future commits. Not sure what else you would expect me to do?

[olexandr-konovalov]

If you have (or will have) a place somewhere in the documentation and will explain how to contribute, you may state it as rule there.

I do not agree that this is as trivial as writing "do not to let bugs slip through in future commits". Many GitHub users do like to use Closes #XXX in commit message to close the issue from commit, so there is a real chance that someone will make a commit with it, and someone else will review and merge a commit with it...

[fingolfin]

I don't believe that adding this to the README would have any effect. Think about it: I just closed issue #6 in which you requested in 2017 an addition to the README which actually had been added in 2014. Fact is: People don't read READMEs, or at best skim them, and they certainly don't expect the README to contain instructions about things that are forbidden for commit messages. So having this in the README or anywhere else will have zero effect.

The only thing that would possibly help is a commit hook resp. a bot which checks all PRs for commit messages containing "close #XYZ" messages. If somebody wants to set up the latter, that'd be nice, but I won't spend energy on it (I got exactly one tiny README update as a contribution to this project so far, so I don't think spending my time on this would be a good idea).

[olexandr-konovalov]

Same story at https://github.com/carpentries/styles/issues/292. See it for a possible commit hook.

[fingolfin]

For the record, I have now rewritten the history of this repo to remove the three offending commits.

My reasoning are as follows: They are very painful for every new user of GitHubPagesForGAP. On the other hand, very few people ever bother to update to a new GitHubPagesForGAP revision, so they won't notice the rewritten history. And those few who do, I am happy to personally assist.

As to commit hooks: Those are client side only, so at best, if I use them, I won't make that mistake (but I won't make it anymore anyway); while new PRs would not be checked, and could still introduce them. So the only way to get this right would be to have a Travis job which checks incoming PRs for this problem.

[olexandr-konovalov]

Thanks @fingolfin - do you recommend to update this for each package that uses them? It seems not needed to me, since this is only happening when one creates a new webpage for a package, not when updates it. Or do I miss some scenario?

[fingolfin]

No package needs to update for this, but possibly other new stuff here may make it interesting. But unfortunately, any update for existing gh-pages is now mor difficult. I will look into providing instructions for that.

[olexandr-konovalov]

I think that will be useful. For example, having a suggested citation is nice.