-
Notifications
You must be signed in to change notification settings - Fork 20
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(mojaloop/#2092)!: prefixes for api definitions and upgraded node version #138
feat(mojaloop/#2092)!: prefixes for api definitions and upgraded node version #138
Conversation
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.
Please some comments.
In addition:
- please add
BREAKING CHANGE: <REASON>
to your PR description & commit message (i.e. changes to Dockerfile/opt/app
working dir, etc) - can you align the folder structure to the general pattern we follow at Mojaloop for consistency, please? I.e. /src, /test etc at root level.
- please add CODEOWNERS as well, ref.
- Please add the following to the end of your README.MD
a. auditing-dependencies: https://github.com/mojaloop/central-ledger/blob/master/README.md#auditing-dependencies
b. container-scans: https://github.com/mojaloop/central-ledger/blob/master/README.md#container-scans
c. automated-releases: https://github.com/mojaloop/central-ledger/blob/master/README.md#automated-releases
Take note that you should create a snapshot for testing on the OSS environment before any merges are done here, example PR where this is done: mojaloop/als-oracle-pathfinder#72
Co-authored-by: Miguel de Barros <miguel@debarros.me>
Co-authored-by: Miguel de Barros <miguel@debarros.me>
Co-authored-by: Miguel de Barros <miguel@debarros.me>
BREAKING CHANGE: upgraded node version to v16
…com/vijayg10/ml-testing-toolkit-ui into feat/implment-prefixes-for-api-defs
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.
Hey @vijayg10,
One minor comment left for you to address.
@vijayg10 I also updated your PR description, please take note of it, and ensure that you include the PR description as part of your merge commit message. We need to be specific about what is breaking! Node upgrade is nice, but the real breaking change here is the dockerfile working directory that needs to be explicitly called out. Unless there is something else that you are aware of that needs to be added. |
I further update the PR description. See the changes above. |
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.
+1
feat(mojaloop/#2092): upgrade nodeJS version for core services - mojaloop/project#2092
Notes:
/opt/mojaloop-testing-toolkit-ui
to/opt/app
as follows:BREAKING CHANGE: Major version bump for node v16 LTS support, re-structuring of project directories to align to core Mojaloop repositories and docker image now uses
/opt/app
instead of/opt/mojaloop-testing-toolkit-ui
which will impact config mounts.Major version bump since this is a big upgrade.