#3789 - Git policy changes
-
By Chris Graham
- Added
- 6 views
| Identifier | #3789 |
|---|---|
| Issue type | Feature request or suggestion |
| Title | Git policy changes |
| Status | Completed |
| Tags |
Roadmap: v11 (custom) |
| Handling member | Chris Graham |
| Addon | General / Uncategorised |
| Description | Implement a number of changes around our use of git to improve communication for people.
Problems being solved (from a human perspective): i) People currently may struggle to trace issues (especially major bugs or security holes), due to messy commits. ii) There are no formal changelogs, just messy commit messages and spotty coverage on the tracker. People want to know what's going on, to see progress and see if they need to upgrade. iii) There are no documented standards for our use of git. Programmers may not know what to do. Programmers may not know how to maximize their effectiveness. iv) Github does not represent our ideals. The Github code is not itself Open Source (=hypocrisy), and it is owned now by Microsoft who has traditionally undermined the work of Open Source developers (and still does in some ways). Certainly Windows is not Open Source, so why should Open Source developers be handing out Open Source code in a way that benefits Microsoft. Historically Composr has not done a good job of communicating its deep-rooted Open Source idealism: we need to make sure we don't align ourselves with anything that muddies perception of what we believe in. Specific technical problems: I) Commit messages may not be well worded, and it is not possible to edit them without "git push --force", which is not a good thing to do. II) Multiple different issues may be done within single commits, which is very messy III) Git commit messages and tracker issues are rarely linked, and tracker issues are usually missing IV) Discussion may end up on Github commits instead of tracker issues Solutions... 1) Improve commit discipline Document coding standards for use of git in codebook-standards.txt (and make some platform changes to provide tooling to support the new policies): a) Any commit on a stable branch should fix one issue only Tip: use the git patch technique (https://stackoverflow.com/questions/1085162/commit-only-part-of-a-file-in-git) b) The message for any commit on a stable branch must either be "Fixed #<tracker-ID>" or "Implemented #<tracker-ID>" or "Security fix for #<tracker-ID>" or "New release" (for updating metadata files around a new release). Multiple commits may reference the same tracker issue. Reason: So that the tracker and git are bound together. Reason: So that typos can never happen in git commit messages, given all the custom text is in the tracker issue. Tip: Use the "push bugfix" feature in Composr composr_release_build addon to automatically create appropriate git commit messages and create tracker issues, and hotfixes Platform change: A git pre-commit hook will check the commit naming rule. Platform change: Create an XML file in the composr_homesite branch which defines the status of the different branches. For each git branch representing a version, define the Composr version (e.g. 10.x.x), the planned EOL date, and the current state (e.g. beta). The XML file will be used by a page of the website to display version statuses, and also to power the above git hook. Where the website talks about version statuses we can also link to agencies willing to support these versions (agencies can offer support contracts whereby they do whatever patching and patch backporting may be required, probably for a fixed monthly fee). Platform change: Add dev mode check that the web server is running as the same owner of the git repository, to make sure file permissions don't get messed around with when executing git-hooks or doing the push_bugfix module (which runs git commands) Platform change: Enable Git integration on the tracker https://support.mantishub.com/hc/en-us/articles/360008778613-Setting-up-Source-Control-Integration Platform change: Improve hot-fix generation in the push_bugfix script so that it can followup any previous hotfix with a new one (with an updated filename), merging in any files from the previous one that were changed and not covered in the new commit Platform change: Improve hot-fix generation in the push_bugfix script to have a forced checkbox confirming that the developer manually tested the fix worked Platform change: Adjust the push_bugfix to have radio buttons to choose the issue type (bugfix vs feature vs security) Platform change: Make push_bugfix automatically edit any existing issue referenced on the form so that the version field and severity field are at least not contradicting the version the fix is being committed for and the issue type mentioned above c) Commits on an unstable branch should generally have a tracker issue, but don't need to be linked to it explicitly Reason: To maintain agility and developer happiness when working on major updates we need to keep things streamlined - but we also need to be able to easily build changelogs for major new releases. d) Doing lots of small changes in one commit is not prohibited in an unstable branch Reason: In the real world there will be rapid development and heavy lifting going on for major updates and having atomic-level commits would cause massive revision history and burden. It is not necessary for solving any of the identified "human perspective" issues, which revolve around a need for being conservative on stable branches. e) Use your personal preference for merging - merge or rebase or rebase with squashing Reason: Git rebase is technically more complicated to perform, so is a burden, and it doesn't provide substantial advantage. Some developers may prefer it though (it is a bit cleaner), and it doesn't cause any harm. Git history is not too hard to read in either case if using a tool with good commit graphing support (gitlab and command line git can do it), or if reading commits from "git log --first-parent". f) If working on something for extended periods of time (days or longer), use a git branch for it. That branch may or may not be pushed upstream, this is your choice depending on whether collaboration is required while it is developed as a separate branch. Reason: Entanglements occur when multiple developments are happening in parallel (changes to the same files), which is unavoidable when working over an extended period of time. The isolation capabilities of branches makes things much more manageable with only the small overhead of creating and merging these branches. Reason: Being able to make regular commits is useful for many reasons, but if it's not on a branch it looks like a messy chain of consciousness scattered within a wider sea of other work. The aforementioned reasons include getting backups, delineating (logging) clear steps within the development, and being able to share your progress with others at relatively stable stopping points. g) Include any officialish non-bundled addons within the main repository Reason: We don't use submodules because this is generally complex, and in particular would require isolating addons under particular code paths, which works against Composr's approach of tight-integration. Any mitigation of this (e.g. symlinking) would add undesirable complexity at many points. It is conceivable that if/when the large sea of addons are developed by reliable individual maintainers, those addons could be fully managed from their own repositories; at the moment the overheads in that are not desirable. 2) Switch to Gitlab Switch from Github to Gitlab. Gitlab is truly Open Source, also free to use for OSS projects, and in some ways better than Github. Typical objections regarding migrating from Github are that people would have a harder time finding Composr - but most people do not find projects by searching Github. 3) Disable comments on Gitlab I'm not sure if you can do this. You can do it on Github. If it can't be done, put through a feature request to Gitlab. 4) Automatically create changelogs for new release news posts Add in a search link when our release scripts create the release news posts. 5) Re-visit security disclosure policy This policy was originally written when we used subversion on a private server (not git). Keeping any issue secretive once a fix is put to git is not possible, assuming people look at changes made on git. In terms of sitting on issues until a fix can be made, this is not something we are likely to do -- we resolve fixes quickly and don't want to risk users being exposed to an issue private hackers may also be aware of. If there's a concern of getting people updated quickly then this is a commercial consideration that should be met with some kind of support contract between user and any company wanting to commercially support Composr. Likely the policy can just reflect what the normal commit-based behaviour would be, in line with what is discussed in this issue. Then no developer has to think of that policy. |
| Steps to reproduce | |
| Funded? | No |
| Commits | Fixed MANTIS-3789 (3718f6e6) · Commits · Composr ecosystem / Composr · GitLab |
The system will post a comment when this issue is modified (e.g., status changes). To be notified of this, click "Enable comment notifications".
Comments
Everything is implemented, although some of the details differ to what the issue says.
A lot of improvement was made to the Push Bugfix minimodule that is available to help developers and improve how issues are fixed by generating automatic hot fixes, and also changes (simplification) to the Mantis bug reporting options. You can now set the bug severity in the minimodule. You can now use it from any admin username & password, you don't need a master password.
GitLab has indeed been integrated with Mantis. To do this, commits need to contain MANTIS-<issue-ID>. GitLab will link through to Mantis (which is awesome). Mantis will only link to GitLab by virtue of the posts made by the Push Bugfix minimodule, Mantis itself has no direct integration back to GitLab, but that does not matter.
I cannot disable comments on the commits as they show on GitLab. However, I think as the Mantis issue is clearly linked people will generally comment on that issue. If they comment on GitLab, no huge deal - we can just live with it, or educate the user.
People may question why we don't dump Mantis and just use GitLab. Apart from that being extra work to migrate, I think it is good that we have a more powerful customisable system that is integrated well with GitLab and the compo.sr site.
I've allowed commit messages with "Implementing" as well as "Implemented", as often a developer may have to come back to tweak changes previously made and there needs to be a way of saying that.
If you use a commit message that doesn't match the standard, it'll tell you what the standard is. It isn't annoying to deal with.
Developers do need to make sure +x is set on all the git-hooks/* files for git to enforce the commit message standard.
When it comes to the automatic changelogs for new Composr releases, it doesn't say fixed/implemented/etc, it just shows the issue titles from Mantis and links to Mantis. If multiple commits refer to any particular issue then only one changelog line will be present for them. I have also allowed commits to provide additional text, as I realise more explanation may be desired for followup commits than just linking to a tracker issue can provide. The changelogs are also fully autogenerated, no need for a 'search link' or anything.
The next patch release will have a mixture of the changelog style that launched for the last patch release (which is just linking direct to git commits), and the behaviour described in the previous paragraph. This is due to it existing over the transition period. In the further future changelogs will only link to git commits if it cannot match it against a Mantis issue, which should not happen.
I haven't implemented version maintenance policy declaration via an XML file, as the potential of having different versions of that file across different branches would be chaos. Instead I've made a block that can scan all git branches that have a version-branch naming convention, identifying the maintenance status of that branch. This works by an extension to what already was in version.php. i.e. the version.php file of any git branch defines whether that branch is still being maintained, and what kind of branch it is (alpha, beta, etc). The tradeoff is we need to maintain API compatibility on version.php, but I think that'll be fine, or workaroundable at least.
Rather than a dev-mode check to ensure the web server is running as the owner of the Composr files, I've just put that check in the Push Bugfix module. It doesn't need to be everywhere.
I would note that it is good that we finally got Git policy into our coding standards document. Somehow this fell through the cracks. Most of that document was done when we still used Subversion.
Rewriting the security policy was important, and something we should have done ages ago. Frankly, we haven't been properly following everything it said as it was seriously outdated for the modern world and context of the project. I've done this unilaterally, but of course people can review what I've wrote and chip in -- it's at the bottom of https://compo.sr/docs/tut-software-feedback.htm
This fixes many small issues in the push_bugfix script:
- No git add for new files
- Problems in filesystem search
- Needs control of whether to post hotfixes
- Needs control of whether to close issues
- Better git command debugging (HTML comment for git output)