? NPM Resource Changed PR-4.3-dev Pending

User tests: Successful: Unsuccessful:

avatar heelc29
heelc29
27 Aug 2022

Summary of Changes

With PR #38422 the versions of codemirror and tinymce was downgraded:
image
image

This PR updated these extensions back to:

Testing Instructions

code review

avatar heelc29 heelc29 - open - 27 Aug 2022
avatar heelc29 heelc29 - change - 27 Aug 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Aug 2022
Category NPM Change
avatar richard67
richard67 - comment - 27 Aug 2022

I've just checked and can confirm that this PR here is complete, i.e. the version numbers in files "plugins/editors/codemirror/codemirror.xml" and "plugins/editors/tinymce/tinymce.xml" were not downgraded by PR #38422 and so still are right.

@brianteeman Could you have a look, too? Maybe I'm missing something?

avatar brianteeman
brianteeman - comment - 27 Aug 2022

is this even needed. It is for the 4.3 branch and when the next upmerge is done it will be resolved. And hopefully then it will have the latest updates merged.

Note: When you want to update a package you only do npm update package and not blindly npm update everything
Maintainers whenever there is a pr with a package-lock update you need to take extra care to make sure the submitter didnt make this mistake.

avatar richard67
richard67 - comment - 27 Aug 2022

It is for the 4.3 branch and when the next upmerge is done it will be resolved.

@brianteeman Not necessarily because the PR which downgraded the versions on 4.3-dev was committed after the ones which upgraded them on 4.2-dev, so there will not be any conflict on the upmerge, and git will keep it as it is. So I think this PR here is good to make sure it goes the right way. But you are right when there will be an update on these places in the package lock on the 4.2-dev branch later. Then this PR here will not be needed anymore because that later change would go up with the next upmerge after that.

avatar richard67
richard67 - comment - 27 Aug 2022

Note: When you want to update a package you only do npm update package and not blindly npm update everything Maintainers whenever there is a pr with a package-lock update you need to take extra care to make sure the submitter didnt make this mistake.

Fully correct. The mistake has happened when PR #38422 was merged, and that PR changed also other dependencies which should be checked. When doing like @brianteeman said, the package lock should show only expected changes for the updated package and its dependencies.

The same applies to composer dependencies.

avatar richard67
richard67 - comment - 27 Aug 2022

I have tested this item successfully on d9df238


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38614.

avatar richard67 richard67 - test_item - 27 Aug 2022 - Tested successfully
avatar heelc29
heelc29 - comment - 27 Aug 2022

and that PR changed also other dependencies which should be checked

hotkeys-js was updated to 3.9.4
image

For information: I updated the lockfile with npm install codemirror@5.65.6 and npm install tinymce@5.10.5 and reverted the changes of semantic versioning under dependencies

avatar viocassel
viocassel - comment - 28 Aug 2022

I have tested this item successfully on d9df238


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38614.

avatar viocassel viocassel - test_item - 28 Aug 2022 - Tested successfully
avatar richard67 richard67 - change - 28 Aug 2022
Status Pending Ready to Commit
Labels Added: NPM Resource Changed PR-4.3-dev
avatar richard67
richard67 - comment - 28 Aug 2022

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38614.

avatar obuisard
obuisard - comment - 31 Aug 2022

Thanks @heelc29. Can you update your branch so I can merge your changes? Thank you!

avatar brianteeman
brianteeman - comment - 31 Aug 2022

@obuisard you should have the rights to update the branch.

avatar obuisard
obuisard - comment - 31 Aug 2022

@obuisard you should have the rights to update the branch.

Hi Brian, it does not seem like I do, unless it is possible other than from the Github interface.

avatar brianteeman
brianteeman - comment - 31 Aug 2022

@obuisard then you need to speak to the other maintainers as they all can. Its crazy if you dont and it will take forever to get things merged. (unless the user has marked this as cannot be updated - can you update other PR?)

avatar obuisard
obuisard - comment - 31 Aug 2022

Actually, you are right, I can update branches on other PRs, just not the ones from @heelc29. Thank you Brian.

avatar heelc29 heelc29 - change - 1 Sep 2022
Labels Added: ?
avatar heelc29
heelc29 - comment - 1 Sep 2022

Thanks @heelc29. Can you update your branch so I can merge your changes? Thank you!

@obuisard Done 😀

avatar obuisard obuisard - change - 4 Sep 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-09-04 01:05:39
Closed_By obuisard
avatar obuisard obuisard - close - 4 Sep 2022
avatar obuisard obuisard - merge - 4 Sep 2022
avatar obuisard
obuisard - comment - 4 Sep 2022

Thank you @heelc29 for the PR :-)

Add a Comment

Login with GitHub to post a comment