Replies: 42 comments
-
Definitely a good idea although I think CHANGELOG.md would be more user-friendly it would also be more annoying to maintain so I'm open to either proposed method. |
Beta Was this translation helpful? Give feedback.
-
I support this idea, as long as we dont require it for PRs. The more problematic part is removing stuff which got fixed upstream by magento |
Beta Was this translation helpful? Give feedback.
-
Agreed, it shouln't be a requirement . I'd prefer the "easier to maintain" solution. Add a first label for kind of this PR (bug/patch/...). I also wouldn't remove entries that got fixed in later Magento version. It should be enough to add a second "branch-label" that match magentos versions, where the issue was fixed. |
Beta Was this translation helpful? Give feedback.
-
Don't know where to ask ... ... is there any "schedule" for merging approved PRs? I don't want to stress someone, but there are some PRs that should go live ASAP .... (i know i could use a/my fork, but i definitly want to rely on "official" LTS) |
Beta Was this translation helpful? Give feedback.
-
+1 for making human readable changelog. Git history is hard to read because we use non linear history (merge commits) and commit descriptions/titles are also not too nice. List of merged PR's is also not a solution (in the form it is right now) as you don't know which PR was applied on the current branch. I think we should require every PR to add appropriate changelog entry. |
Beta Was this translation helpful? Give feedback.
-
But I would like to see references to Issues in commit messages |
Beta Was this translation helpful? Give feedback.
-
@Flyingmana can you please share the command you will use to generate a changelog based on the git log? |
Beta Was this translation helpful? Give feedback.
-
I "slightly" improved |
Beta Was this translation helpful? Give feedback.
-
+1 for the one who added some labels ;) |
Beta Was this translation helpful? Give feedback.
-
See below for example changelog generated form git history:
In my opinion we should either improve pull requests:
Or maintain the changelog manually.
|
Beta Was this translation helpful? Give feedback.
-
I've just merged a pull request in a slightly different way then before (204cb68) How do you like it? |
Beta Was this translation helpful? Give feedback.
-
So if we are going to use this method to maintain a changelog going forward should we just always squash PRs when merging and clean up the commit message to be descriptive but succinct at the time of merge? I've not tried this but it seems it could be easy and still yield good results. |
Beta Was this translation helpful? Give feedback.
-
I just tested the "Squash and Merge" method on #242 and here is what the result looks like on the commit history with yesterday's merge for a comparison. It automatically added the PR # to the commit message which wasn't there before and you can modify the message and the longer description. Seems pretty solid to me but this is just one single-commit PR so I don't know what other issues there could be. It does seem to maintain the original committer "credit" though which is definitely important. |
Beta Was this translation helpful? Give feedback.
-
Ignoring all other discussion in this thread, I'm 100% against squashed merges. I think if we or the contributor prefer less commits in the log, then that should be done prior to issuing the PR. |
Beta Was this translation helpful? Give feedback.
-
maybe there should be contributing guidelines. only one commit by squashing before the PR. the commit should describe what has changed/improved/fixed and why. a signature line - who is the author of the commit (name and email) and maybe a line with tested magento version (for later reference) and maybe a list of people having approved the commit. |
Beta Was this translation helpful? Give feedback.
-
Just few notes from my experience as a core team member of TYPO3 - a top CMS in DACH market, which has 150-200 commits per month and 30-50 contributors per month (many of who are first time contributors). We also maintain 3 versions(branches) in parallel. How TYPO3 does itTake a look here: It's is super easy for everybody to understand what given commit is doing
We squash every branch so it finally lands as a single change in the product branch. If you want to see the evolution of the patch, you can always find it in the pull request history. But to be honest it's you need it very rarely. There are also some other rules, like - every breaking change, deprecation or a new feature requires a simple documentation file like this TYPO3/typo3@fe563ff A core developer who is merging the change is responsible of checking if the commit message is correct and describe the change in a correct way. It is neither hard nor time consuming. GoalsThe changelog/git history and the readme should:
Proposal for MageLTSThis project is of course smaller and has less contributors than TYPO3, but I think we can adapt some of the concept with big benefit for us.
|
Beta Was this translation helpful? Give feedback.
-
This is the only point I'm not sure if I agree on (applying all fixes over a new branch). Users should be able to maintain their own branches and pull in upstream changes easily and I'm not sure how this works with the suggestion made. Also, there will be conflicts with upstream and I think the task of resolving these conflicts each time a new upstream version is released will be a lot of work and prone to errors each time they are resolved. That is, once a conflict has been resolved I don't see why we'd want to have to resolve it ever again. If there is ever another "major" upstream release (e.g. 1.10.0.0 - I don't expect there to be) then at this time we can decide if we should start a new 1.10.0.x branch with all patches rebased on it, but I think for now we should focus on maintaining the 1.9 branch in the simplest way possible. @Flyingmana Since we are using Github the full history is available in the Pull Request and doesn't need to be in the main branch history. Case in point, this PR resulted in 2 lines changed and 7 commits and although I squashed it when I merged it, you can still go back to the PR that is referenced in the commit message and see the full discussion and 7 commit history in all it's sloppy glory: https://github.com/OpenMage/magento-lts/pull/388/commits |
Beta Was this translation helpful? Give feedback.
-
While it would be the cleanest and safest way to handle the upstream process you certainly have a point. I think what @tmotyl meant was to use this as verification before a larger upstream commit is merged into 1.9.3.x. Especially if there were any conflicts.
Yep, as I said previously, for this to be feasible a clean git history is pretty much required. The heavy lifting for this has been done in my fork and I will be able to maintain it as part my of work, so this could be used for such occasions. Maintaining the clean fork/branch could also be automated with webhooks and a bit of shell scripting if patches/upstream commits are tagged accordingly on 1.9.3.x, just cherry pick and push. For a new Magento release the process boils then down to (using 1.9.3.7 as example):
Then just run meld, diff or whatever to see if Doing this process before applying the upstream changes to the 1.9.3.x branch should also give a heads up if extra care is needed not to break any patches while doing the merge. If the patch set can be applied without conflicts the merge on 1.9.3.x can be treated as one way: |
Beta Was this translation helpful? Give feedback.
-
Are you saying that at the end of creating the new 1.9.3.7 branch you would merge this new branch into 1.9.3.x? There are many ways to do this so please give a specific example of what you recommend. E.g. is it something like:
I've not tried the above but I think it would take the result from your ground-up method and apply it as a new commit to the existing branch. |
Beta Was this translation helpful? Give feedback.
-
I'm not dogmatic on the point 5), probably we need to battle test it to see whether our assumptions are correct. Having a clean git history also allows us to spot bugfixes which were also fixed in Vanilla Magento but in different way (differnt place) - in that case we would probably need to remove (skip) our patch. |
Beta Was this translation helpful? Give feedback.
-
After taking some more time to think about it, and also realizing this Topic is now already going on for a year, I suggest we should find a way to start having a changelog and improve it incrementally then. Also I will change my position on the Topic of stashing. If we notice in another year, that it is not working out we have at least proper reasons to refer to. And overall it seems here are enough people with more experience then I have on this topic, therefore I should step back a bit and not hinder you. Also I have to apologize, my last comments were to harsh and could be seen as harmful, Iam sorry for this. |
Beta Was this translation helpful? Give feedback.
-
Yep. Anything i till better the nothing :P |
Beta Was this translation helpful? Give feedback.
-
Sorry for being unclear, no I wouldn't merge the diff directly into
As of
In terms of automating the "patch set" branch this would be guaranteed to work as both branches should be at all times identical. The webhook just cherry-picks everything but upstream merge commits from If a new Magento version is released a new branch is started for the patch set. I'm not sure this part can be fully automated, but worst case shouldn't be more than executing a shell script Regarding the change log, if a clean branch is maintained it could be used to generate the change log from scratch, the webhook maintaining the branch could also take care of pushing the updated change log to
So let's start with defining how commit message formats should look like and get this thing rolling, proposal:
Project related commits like updating the
|
Beta Was this translation helpful? Give feedback.
-
Just a note to commit message format ...
see: https://help.github.com/articles/closing-issues-using-keywords/ How about ...
|
Beta Was this translation helpful? Give feedback.
-
Ah, good point, didn't think of that. It's not really relevant for the change log though and more importantly steals precious space from the commit message. How about adding that kind of stuff below the commit details instead?
My reasoning here was that there is most likely always a PR, but not always an issue for each PR. It may also happen that a PR has no reference to it's issue. Fine with either though. For bugs it would be nice to have the first Magento version that introduced it in the message header, should be optional though:
|
Beta Was this translation helpful? Give feedback.
-
Changelog was already present in the https://github.com/OpenMage/magento-lts/blob/1.9.4.x/RELEASE_NOTES.txt file, Magento later replaced the way the changelog was shows (just link an online version of the release notes) but I guest that could be a nice place to store it. |
Beta Was this translation helpful? Give feedback.
-
This issue is 4.5 years old... it's time to decide something or close it |
Beta Was this translation helpful? Give feedback.
-
it kind of is already decided to have a changelog. But we still need something to provide a changelog for the what happened before we started writing one for new releases |
Beta Was this translation helpful? Give feedback.
-
Hi everyone! In my company, I asked to use some conventions like
Moreover we use the Python CLI named Changelogdir to auto-generate our changelog by CI. |
Beta Was this translation helpful? Give feedback.
-
I like the Conventional Commits proposal, very easy to adopt, although not as easy to enforce. The changelog options both have downsides.. single file has conflicts and multi-file means moving files around to the right version (e.g. backporting is more work). Just doesn't seem like a great solution for a Github-based project. However, by adopting Conventional Commits it would be easier to script a changelog generator (surely one already exists) |
Beta Was this translation helpful? Give feedback.
-
Well ... why?
In my opinion the actual description does not point out what the advantages of Magento-LTS vs Vanilla Magento are. Uhhm ... OK ..."They accecpt bug fixes". Sounds not so bad, but it's not clear whats already in ...
There are
Why dont tell that - to make this project more attractive?
Cause i'm lazy i wouldn't add a CHANGLOG.md ... i would add a link to README.md that points to https://github.com/OpenMage/magento-lts/pulls?page=1&q=is%3Apr+is%3Aclosed+is%3Amerged&utf8=%E2%9C%93 and add some labels for all merged PR (bug, enhancement, patch ...)
Beta Was this translation helpful? Give feedback.
All reactions