The purpose of creating an issue in the bug tracker was to link the change such that it’s possible track the status of the pull request against 1.3.
In terms of the first 2, that was more from the fact that I’d initially fixed them separately years ago, so hadn’t spotted I’d done the same thing as two separate fixes in the past.
We can always choose not to link the issue to the change log (i.e. not set a fixed in version variable) for these sorts of changes, but we shouldn’t delete issue reports as that removes history permanently – it’s not exactly an issue to have an extra row in a database table.
A pull request in github should have a related Mantis bug tracker issue to track. Ideally, IMO – we should have the discussion around the issue in our bug tracker rather than on github (after all, we are coding a bug and issue tracking with source code integration). Equally, if it was an end user submitting the improvement, we tell them they can create an issue and upload a formatted patch.
Whilst in this case, I’ve used github for the pull request, I could equally have just added the same patch to the bug tracker or emailed it to the developers list instead. (From a personal point of view, it would have been easier to have generated a diff containing these 3 changes then a new branch in git as I already had the 3 diff’s.)
So, in short, I think it’s important that we have issues in our bug tracker for any pull requests. From the discussion that’s happened around pull requests, in terms of improving the quality of the code, we seem to be having some beneficial discussions in terms of improving code quality.
At the same time, the issues you link are rather trivial, and we are now holding up development for a week (or at least until everyone gets time to review the change if earlier) for the review process to take place.
In short, given our current review process, these should exist as bugs in the tracker – we probably do need to consider not applying the fixed in version tag for some fixes so as to keep the change log readable.
For instance, lets take 17377 / 17378 as an example. Before, I’d started doing work to reduce the number of global variables in Mantis. These commits are backports of this work. (The language file changes that we did, then didn’t include or may have left in the api – I think revert then unreverted them so I can’t remember where we left them, was also part of this). The purpose for reducing the number of global variables in my eyes was really to make it easier for people developing around Mantis by reducing the amount of items in the global state, whilst also potentially making it harder to exploit/break something by using the global state.
Anyway, in this case, I’d say the correct behaviour is to create a bug (for the changelog purpose),
“Reduce PHP global state” -> this bug would have fixed_in_version set to 1.3.
Issues 17377 and 17378 would then be set up as related (child) bugs to “PHP global state”, and when marked as resolved, we’d *not* set a fixed_in_version value.
This enables us to provide a more user-friendly changelog, whilst keeping a complete history.
We should probably apply this process to some of the other issues in the changelog – for example, we could simplify the display of the improvements to oracle in the 1.3 branch using a similar technique
I thought it is worth discussing this so that we have clear process for it.
http://www.mantisbt.org/bugs/view.php?id=17378 – move a variable from global to static.
http://www.mantisbt.org/bugs/view.php?id=17377 – move another variable from gobal to static.
http://www.mantisbt.org/bugs/view.php?id=17376 – se sprint instead of utf8_str_pad for perf
These are all fine issues that we can fix, but having them as issues in the bug tracker and eventually in the change log is almost spam. Also having two separate pull requests for each variable we want to rename seems to be an overkill.
Having said that, other issues like “remove copy_fields utility” make sense.
I suggest, we delete the 3 issues referenced above. However, the changes can still go in. We can continue to use the code clean category to track bigger changes.