Hi Gramps devs,
Over the last couple of months I’ve watched PRs for gramps 5.1.x and master being merged by @Nick-Hall and prepared for release. However, I’m unclear on how bug fixes for the add-ons project are merged, either maintenance and/or master.
What’s the next step after a PR is reviewed? PR approval, merging, building of the add-on, upload etc. Addons Development wiki has good info, but nothing about this part of the process.
Thanks
Only extra info I can add is what the README.md file shows:
- For the developer who is merging PRs or other commits and needs to rebuild and list one or more addons
python3 make.py gramps50 as-needed
Addons don’t follow the release cycle for core Gramps code. Most updates will occur in the current maintenance branch (gramps51 at the moment). When the author is happy with a change, it can be merged into the addons-source repository. All addon authors can have write access to this repository.
A new version can then be built using the make.py script. This creates a new version of the archive (tgz) file and a corresponding entry in the listings. Only trusted developers can actually publish an addon. We will quickly review the submission for malicious code before merging into the addons repository.
The addon author is responsible for reviewing code, and deciding the direction of development. If the original developer is no longer interested in maintaining the addon, then we may have to assign a maintainer.
Thanks, @Patsyblefebre and @Nick-Hall, that all makes sense. A couple of clarifications:
In this context Is the “author” the original author/designated maintainer of the addon, or could another contributor, for example someone who fixes a bug, merge the PR?
Currently I have a couple bug-fix PRs open which aren’t reviewed although they have some thumbs-up reactions. Should I attempt to identify the addon author and at-mention them for a review to move the PRs along?
As far as building the addon: while I tested the bug fixes, I don’t have a full gramps development environment on Windows so I can’t yet build the addons and follow-through to deployment. So I’m hoping that the author/maintainer can take care of that.
List of all open PRs against addons-source:maintenance/gramps51 branch
The add-on list wikipage lists contacts (or suggests filing a MantisBT bug report / feature request) for 3rd party add-ons. And the .gpr.py file for most add-ons and the combined .gpr.py file for many of the built-in plugins (gramplets.gpr.py, quickview.gpr.py, view.gpr.py, etc.) often have contact info.
If you’ve hacked an add-on (3rd party or built-in), which method is the most likely way of getting a revision adopted & into distribution?
Sending an email to the Add-on maintainer
Filing a bug report in MantisBT
Filing a Feature Request in MantisBT
Creating a GitHub repository with a fork
Posting a thread in the Discourse Developer section
Posting a message to the Gramps Developer maillist
Create a GitHub Pull Request
Other??
Good questions, @emyoulation. Many of those are a “must” e.g. filing a bug report, creating a PR, etc. I’d love to hear from the developers what makes it easiest and document that as good practice.
What I’d like to propose to the Gramps developers is the use of Code Owners feature in GitHub. In summary, this file in the repo would contain a list people who will automatically be pulled into code reviews when file(s) matching patterns are modified.
This eliminates the need for a contributor to look up the list of addons to find the author’s email to request reviews. Automatically tagging authors/maintainers for reviews makes it so that they don’t have to look for changes they’re interested in. Since Mantis doesn’t alert authors to bugs in addons, this would ensure they do get notified about proposed changes.
Adopting CODEOWNERS can be fairly easy because as @emyoulation pointed out, we have a list of addon authors so it is a matter of assigning folders to authors. One thing that would be helpful is each author’s GitHub handle to avoid exposing their email addresses in the file.
What does the Gramps developer community think about adopting this proposal?
Apologies for being extremely busy and not assisting with Gramps lately. In the last few years I have accepted many PRs for Addons and built/released them. Since i have been busy, this has fallen by the wayside.
I still think that using the PR system is the right way to submit changes to addons. It may help in a few cases to contact the author of the addon as well. For instance, I have not dealt with changes to the Forms addon since that was originally written by Nick, and I don’t want to cause trouble there.
If the original author has tested and accepts (via comment) the concept of the PR change, it saves time for the developer (me?) when it comes to doing the release. Note that the release process bumps up the version number and this results in a commit, so it is usually better to NOT commit the PR if not doing the release; it would add extra minor commits to the history.
@prculley ,
Your history developing the Plug-in Manager Enhanced has something very unique. It is a 3rd Party add-on which supplants a built-in feature.
Is that possible to do that with the built-in Gramplets? It seems like being adopted as a built-in stunts the evolution of a plug-in. If there is an option to override the built-in with an add-on (instead of the confusion of having 2 features doing nearly identical functionality), then what is involved?
I’ve been corresponding with the 2 authors (Reinhard Müller & Jakim Friant) listed for the built-in “What’s Next?” Dashboard gramplet. Both continue to use Gramps but are no longer actively developing.
With your previous suggestions (thank you!), I have added some Tree Prerequisites tests to that Gramplet. It can now help newbies recognize when no Tree is loaded. Or when no People exist … or no Active Person has been defined. But there are no active maintainers to do validation or commits.
But there is SO much more improvement that is possible. (Like a Preference option to throttle the count of generations crawled to eliminate the awful latency this Gramplet introduces.)
I’m afraid that even if the changes are adopted for 5.2, it won’t see another tweak for a couple of years… until 5.3 or 6.0 is released. If an updated 3rd Party Add-on of “What’s Next?” can supplant the built-in, then that worry can be eliminated.
Can add-on registrations do that? If so, how? I’d like to put the tweaked version out there for 5.1 for feedback.
@prculley Thanks for doing the work of taking PRs through build, packaging and deployment. Now I have a better idea of how this process goes for gramps addons.
Sounds like it’s best to at-mention the authors of the addons for a review of the PR. Once they approve it you can continue with the process when you’re ready for it.
I cannot think of a way to override a standard plug-in like the standard Gramplets. The way plug-ins in general work is that first the standard ones are scanned, then addons. If any duplicate IDs are found this is an error, so you cannot just override by using the same ID.
With the Enhanced Plug-in manager, it was just possible to replace the original because I was replacing whole callable modules; I could modify the Python dictionary of callable modules before they were used by making the dictionary change during the scan for addons via the load on scan feature.
The Gramplets are loaded with a few lines of code mixed in with other functionality. I would have to replace whole blocks of code to make a change. It might not work at all if the block to be changed was loaded before the addon was scanned. Even if it did work, the new addon would make Gramps more fragile, see what happened when the configure dialog was modified and the Styles addon messed up the new dialog changes in the 5.2 development.
It might be better to try to design a standard way to override a plug-in via an addon into Gramps. Maybe float this as an enhancement? I know that some plugins would like to get tweaked faster (Narrative web comes to mind), so this might be relatively popular.
How could this enhancement be floated? My understanding of what is involved is too superficial to do a proper write-up. It is likely that just the way I’d phrase things would sink any useful ideas.
Nick already talked about expanding some of the Gramps plugin registry file features for import into the GitHub addons-__.txt. (like a link to the GUI thumbnail, & wiki page) Some of which may help the wiki’s add-on list become a bit more automated. (Just making help_url universally compatible with all the add-on types would open a lot of possibilities.)
@prculley Appreciate you finding time to merge in PRs in the addons-source repo.
Developers,
To wrap things up and put a nice bow around things, two more bits are still missing from the picture and I’m looking to fill that gap:
The code fix is now in the maintenance/gramps51 branch. Addons wiki says, “The developers are currently merging changes to the most recent maintenance branch into master as necessary, so you don’t have to do anything for that unless you are in a hurry.” There’s no hurry, but I’m curious what triggers a merge of the maintenance branch into master? Does it coincide with Gramps release or some other event?
The Mantis BT issue associated with a bug fix that was merged is still open and based on the wiki, it may be ready to be marked as Status=Resolved and Resolution=Fixed. For addons, should the Target Version and Fixed in Version be filled in with version of the addon which includes the fix, or leave those fields blank for addons?
Thanks.
Typically with the next major Gramps release eg: Gramps 5.2.x