-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat(fiori elements writer): 1924 reuse libs for UI.note annotation #2047
feat(fiori elements writer): 1924 reuse libs for UI.note annotation #2047
Conversation
🦋 Changeset detectedLatest commit: f650597 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…or-ui.note-annotation
…or-ui.note-annotation
…or-ui.note-annotation
…or-ui.note-annotation
…or-ui.note-annotation
…or-ui.note-annotation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor description updates needed
Also please add some unit tests
Co-authored-by: Cian Morrin <[email protected]>
Co-authored-by: Cian Morrin <[email protected]>
Co-authored-by: Cian Morrin <[email protected]>
…or-ui.note-annotation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- code is concise and easy to understand
- unit tests cover the functionality
- changeset ok
- didn't test locally but trust the unit tests
…or-ui.note-annotation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor comments
…annotation' of github.com:SAP/open-ux-tools into feat/fiori-elements-writer/1924-reuse-libs-for-ui.note-annotation
…or-ui.note-annotation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes are clear and covered by tests
Changeset ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @korotkovao
- Code is easy to understand
- Tests added to cover
- Comments addressed
Could you please fix https://sonarcloud.io/project/issues?id=SAP_open-ux-tools&pullRequest=2047&resolved=false&sinceLeakPeriod=true? Otherwise, it is ready for merging |
…or-ui.note-annotation
…or-ui.note-annotation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-approving
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-approving
Thank you @devinea @tobiasqueck @IainSAP @cianmSAP |
* origin/main: chore: apply latest changesets fix: i18n bindings validation fails for nested *.properties files (#2021) chore: apply latest changesets fix: Release version of @sap-ux/project-access with filterDataSourcesByType (#2114) chore: apply latest changesets fix: Incorrect change mapping for 'reference' property for Adaptation Project's writers (#2107) chore: apply latest changesets fix: cpe error message regression (#2105) chore: apply latest changesets feat(fiori elements writer): 1924 reuse libs for UI.note annotation (#2047) chore: apply latest changesets fix: add description in i18n.properties (#2086)
* origin/main: (118 commits) chore - Update actions - fix broken pipeline (#2118) chore: apply latest changesets fix: i18n bindings validation fails for nested *.properties files (#2021) chore: apply latest changesets fix: Release version of @sap-ux/project-access with filterDataSourcesByType (#2114) chore: apply latest changesets fix: Incorrect change mapping for 'reference' property for Adaptation Project's writers (#2107) chore: apply latest changesets fix: cpe error message regression (#2105) chore: apply latest changesets feat(fiori elements writer): 1924 reuse libs for UI.note annotation (#2047) chore: apply latest changesets fix: add description in i18n.properties (#2086) chore: apply latest changesets fix: update access modifier (#2103) chore: apply latest changesets fix: Duplicate i18n model in manifest.appdescr_variant for Adp Project (#2101) chore: apply latest changesets fix(axios-extension): parsing of service gen response (#2088) chore: apply latest changesets ...
FEATURE - Automatically enable the notes reuse component if a service is annotated with UI.Note #1924
Check if the provided metadata contains the
UI.Note
annotation. If yes, then addnw.core.gbt.notes.lib.reuse
as a dependency to themanifest.json
.