netcarver Posted December 12, 2017 Author Share Posted December 12, 2017 Hi @adrian on mobile at the moment, I will look into these reports as soon as possible - which will probably be tomorrow evening. Thanks! 1 Link to comment Share on other sites More sharing options...
netcarver Posted December 13, 2017 Author Share Posted December 13, 2017 Hi @adrian 19 hours ago, adrian said: Not sure if you can do anything about this. wireMailSMTP's changelog is not being parsed properly is it? That would depend which page you are viewing it from. If you are just looking at the changelog from the module information page - without doing an upgrade or install, then it looks like it should based on the underlying markdown. If you are seeing this from an upgrade/install page, then I would expect version markers in there. Perhaps I've missed something though, what do you think is wrong with it? 19 hours ago, adrian said: Also, you have a Tracy bd() call on line 702. OT, but any reason you prefer: \TD::barDump() to bd() ? Right, I've removed the call in the next version. Thanks for the catch. I've stuck with using the verbose version of the call as I had trouble with getting the abbreviated calls to work in an earlier (much earlier) version of TD. Link to comment Share on other sites More sharing options...
netcarver Posted December 13, 2017 Author Share Posted December 13, 2017 21 hours ago, adrian said: Just saw this when upgrading AOS. Doesn't seem right. Yep, it wasn't. Please could you try 0.10.3 - I think it should be OK now. 1 Link to comment Share on other sites More sharing options...
adrian Posted December 13, 2017 Share Posted December 13, 2017 29 minutes ago, netcarver said: Perhaps I've missed something though, what do you think is wrong with it? I think it's probably just because the changelog file for wireMailSMTP uses + 0.3.0 fixed code that broke backward compatibility for PW 2.4 and 2.5, brought in with the config cosmetics (0.2.6) + 0.2.7 @abdus fixed smtp password not saving whereas other modules seem to use: ### 1.7.0 (2017-12-12) - fix suppressing the "required" icon in the template editor (by reported by Robin S) ### 1.6.9 (2017-12-12) - PR merge: Different size buttons when editing a page #80 (by gmclelland) 3 minutes ago, netcarver said: Yep, it wasn't. Please could you try 0.10.3 - I think it should be OK now. Yep, looks great now - thanks! Link to comment Share on other sites More sharing options...
netcarver Posted December 13, 2017 Author Share Posted December 13, 2017 RIght, I see what you mean about the format. Hmm. Let me have a ponder about this - I'm not sure I want to support every possible format of changelog, yet if I simplify the regex too much, we may end up with false markup insertions. Anyway, it's not a particularly big problem on the galactic scale of things 2 Link to comment Share on other sites More sharing options...
horst Posted December 13, 2017 Share Posted December 13, 2017 Im not fixed with this format. I can change to the more common format if that helps out. 2 Link to comment Share on other sites More sharing options...
tpr Posted December 13, 2017 Share Posted December 13, 2017 14 minutes ago, horst said: Im not fixed with this format. I can change to the more common format if that helps out. Same here, if someone tells the recommended format for version numbering I'll stick to that in my modules. 3 Link to comment Share on other sites More sharing options...
netcarver Posted December 13, 2017 Author Share Posted December 13, 2017 @horst There's a pull request on your module that should update the formatting of the Changelog to allow it to be parsed by the existing code in my module. I've also fixed a couple of issues with the Markdown in the readme file as well. I hope this helps. As long as the changelog markdown is of the following format, markers should be correctly inserted... # Changelog // this line is optional - not needed, but if it is present it is sliced off and presented left justified above the versions. ## {version string} {optional information, usually the release date} // H2 - github will style this with a bottom border - Change 1 - Change 2 - or - ### {version string} {optional information, usually the release date} // H3 - github will style this without a bottom border - Change 1 - Change 2 @tpr There was a bug in the way I was comparing versions. You should be able to stick with whatever formatting you prefer. That said, I think there is more flexibility with string versioning than using integers. With string versioning you should be able to have deep version if you'd like - "0.7.8.1" or "1.5.2dev" are all possible. I'll be moving my modules over to string versions as and when I update them from now on. 5 Link to comment Share on other sites More sharing options...
adrian Posted December 14, 2017 Share Posted December 14, 2017 Not sure how to handle this (or how important it is), but if a repo has commits that are not tagged with a release, they don't show up anywhere in the "What's Changed". For example I have committed several updates to Tracy that are all about documentation and none of these commits are listed even though they will be included in the user's download if they upgrade. In this case it's fine because none of these affect functionality. What if a dev committed a breaking change but forgot to tag the commit? Should there simply be a warning that there are untagged commits since the last tagged one so you know to take a look on Github, or should those commit log entries be listed above the tagged commits? Any thoughts? Link to comment Share on other sites More sharing options...
netcarver Posted December 14, 2017 Author Share Posted December 14, 2017 Well, I still want to extend the breaking change search to apply to the text of commit messages. Currently the search is only applied to the changelog (if any.) I think extending the search for breaking changes and simply highlighting where a breaking change phrase occurs is probably enough. 1 Link to comment Share on other sites More sharing options...
adrian Posted December 19, 2017 Share Posted December 19, 2017 Steve, what do you think of including "beta" in a version number, eg: '1.2.0 beta' I am wondering about this for both new modules and also those that have received a new feature or some other changes that you think take it from it's listed "stable" status back to "beta". I am thinking this might be good as a warning to users not update on a live site without thoroughly testing their use case. So do you think this is a good idea in general, and do you think it would be worth having this module highlight version numbers with a new beta designation? Link to comment Share on other sites More sharing options...
netcarver Posted December 21, 2017 Author Share Posted December 21, 2017 Hi @adrian Thanks for the idea - I'll give it a try locally and see how it goes. Link to comment Share on other sites More sharing options...
Robin S Posted June 26, 2023 Share Posted June 26, 2023 Hi @netcarver, When ModuleReleaseNotes is installed the main admin headline gets removed in the config screen for ProFields InputfieldCombo and this makes the form layout a bit off too (FieldtypeCombo is similarly affected). Without ModuleReleaseNotes: With ModuleReleaseNotes: I had a quick look and traced it back to the module's hook after ProcessModule::executeEdit but nothing in there leaps out at me as the cause of the problem. Do you have ProFields so you can try and replicate? Cheers, Robin. 1 Link to comment Share on other sites More sharing options...
netcarver Posted June 26, 2023 Author Share Posted June 26, 2023 Ok, it just required a small tweak to the regex that finds the H1 element in the support files (it adds them to that page) and demotes the H1s to styled H2s. In ProField's case, the H1 is not the first line of the readme file - something I was never expecting - so we ended up with multiple H1s in the DOM, with associated styling confusion. Version 0.11.3 should fix it for you. 3 Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now