Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIREQ-1201: Update permission after mod-patron-blocks permission changes #1234

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

Dmitriy-Litvinenko
Copy link
Contributor

@Dmitriy-Litvinenko Dmitriy-Litvinenko commented Nov 27, 2024

Purpose

Update permission after mod-patron-blocks permission changes

Refs

https://issues.folio.org/browse/UIREQ-1201

@Dmitriy-Litvinenko Dmitriy-Litvinenko requested review from artem-blazhko and a team November 27, 2024 12:20
Copy link

github-actions bot commented Nov 27, 2024

Jest Unit Test Statistics

       1 files  ±0       62 suites  ±0   1m 38s ⏱️ -7s
   797 tests ±0     797 ✔️ ±0  0 💤 ±0  0 ±0 
1 106 runs  ±0  1 106 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 9512413. ± Comparison against base commit 1cd124f.

♻️ This comment has been updated with latest results.

Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there an accompanying change to the automated-patron-blocks interface version (or some other interface)? If there wasn't, there should have been 😬 and since you're dropping support for the old permission by removing automated-patrong-blocks.collection.get, that should be mentioned as a breaking change in the CHANGELOG. Alternatively, leave both permissions in place and update the interface dependency.

In NPM-land, leading zeros in a version string cause the minor version to behave like a major from the point of view of semver constraints, e.g. ^6.1.0 will give you >= 6.1.0, < 7.0.0, but ^0.6.1 will give you >= 0.6.1, < 0.7.0. I don't know how we handle this with respect to okapi interface versions, but it feels to me like this should have been accompanied by a change like

      "automated-patron-blocks": "0.1 0.2",

Oooof. There's a lot that's going on with this PR that is outside your control. Given that, and all you can do is react to the eco system you're living in, I will approve this. But I think you should consider leaving both permissions in place in order to maintain backwards compatibilty, even though the interface version didn't change the way it should have.

@Dmitriy-Litvinenko
Copy link
Contributor Author

Dmitriy-Litvinenko commented Nov 28, 2024

Was there an accompanying change to the automated-patron-blocks interface version (or some other interface)? If there wasn't, there should have been 😬 and since you're dropping support for the old permission by removing automated-patrong-blocks.collection.get, that should be mentioned as a breaking change in the CHANGELOG. Alternatively, leave both permissions in place and update the interface dependency.

In NPM-land, leading zeros in a version string cause the minor version to behave like a major from the point of view of semver constraints, e.g. ^6.1.0 will give you >= 6.1.0, < 7.0.0, but ^0.6.1 will give you >= 0.6.1, < 0.7.0. I don't know how we handle this with respect to okapi interface versions, but it feels to me like this should have been accompanied by a change like

      "automated-patron-blocks": "0.1 0.2",

Oooof. There's a lot that's going on with this PR that is outside your control. Given that, and all you can do is react to the eco system you're living in, I will approve this. But I think you should consider leaving both permissions in place in order to maintain backwards compatibilty, even though the interface version didn't change the way it should have.

@zburke Hello "Was there an accompanying change to the automated-patron-blocks interface version (or some other interface)?" no

Was there an accompanying change to the automated-patron-blocks interface version (or some other interface)? If there wasn't, there should have been 😬 and since you're dropping support for the old permission by removing automated-patrong-blocks.collection.get, that should be mentioned as a breaking change in the CHANGELOG. Alternatively, leave both permissions in place and update the interface dependency.

In NPM-land, leading zeros in a version string cause the minor version to behave like a major from the point of view of semver constraints, e.g. ^6.1.0 will give you >= 6.1.0, < 7.0.0, but ^0.6.1 will give you >= 0.6.1, < 0.7.0. I don't know how we handle this with respect to okapi interface versions, but it feels to me like this should have been accompanied by a change like

      "automated-patron-blocks": "0.1 0.2",

Oooof. There's a lot that's going on with this PR that is outside your control. Given that, and all you can do is react to the eco system you're living in, I will approve this. But I think you should consider leaving both permissions in place in order to maintain backwards compatibilty, even though the interface version didn't change the way it should have.

Hello @zburke

  1. "Was there an accompanying change to the automated-patron-blocks interface version (or some other interface)?" no
  2. "automated-patron-blocks.collection.get" added back

@Dmitriy-Litvinenko Dmitriy-Litvinenko merged commit 81dfde1 into master Nov 28, 2024
5 checks passed
@Dmitriy-Litvinenko Dmitriy-Litvinenko deleted the UIREQ-1201 branch November 28, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants