chore: added more documentation to the transform.sh script#16198
chore: added more documentation to the transform.sh script#16198mergify[bot] merged 4 commits intoaws:v2-mainfrom otaviomacedo:transform-documentation
Conversation
|
|
skinny85
left a comment
There was a problem hiding this comment.
Love additional docs! A few small comments.
| # it needs (e.g., jsii-rosetta) for the build. | ||
| # | ||
| # The reason yarn doesn't find the executables in the first place is that they are | ||
| # not dependencies of each individual package -- nor should they be. They can't be |
There was a problem hiding this comment.
This doesn't tell the whole story (for example, Yarn does find the outer monorepo private executables like cdk-build).
Do you know why jsii-rosetta is different than the private packages in tools/?
There was a problem hiding this comment.
Most likely because the script build is just cdk-build, but the script rosetta:extract is yarn --silent jsii-rosetta extract. I tried to remove the yarn part, but that's apparently another can of worms. So it will go to the "ideas to try in the future" section.
scripts/transform.sh
Outdated
| # cloud-assembly-schema and others, which are dependencies of one or more of the | ||
| # tools packages. | ||
| # | ||
| # In addition, we need to copy aws-cdk-lib inside individual-packages/ and remove |
There was a problem hiding this comment.
The tenses here are a little weird.
Are you saying "A possible solution that doesn't involve using this workaround function is to [...]", and then "In addition, we would need to copy aws-cdk-lib [...]", and then "Finally, we would need to restrict [...]"?
In that case, I would change the tenses, and remove the two extra paragraphs (the new paragraphs to me seems to indicate you're tackling a new topic here, which I don't think is the case...?).
If that's not what you mean here, then I don't understand what these two last paragraphs are saying, which maybe means re-wording them a little bit is a good idea 😜.
There was a problem hiding this comment.
Ok, I can move them to a specific section, but they are all parts of the same thing; instead of implementing this stop-gap function, we need to try out a solution composed of three parts: 1) add more packages to the workspace, 2) copy aws-cdk-lib inside individual packages and 3) restrict the build only to the individual packages.
There was a problem hiding this comment.
I created a new GitHub issue, as we discussed yesterday and linked it from here.
Co-authored-by: Adam Ruka <adamruka85@gmail.com>
|
Sorry but I didn't understand the command. Please consult the commands documentation 📚. Hey, I reacted but my real name is @Mergifyio |
1 similar comment
|
Sorry but I didn't understand the command. Please consult the commands documentation 📚. Hey, I reacted but my real name is @Mergifyio |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
I'm not sure whether this is the best place to document these things. I'm happy to move it somewhere else, if someone has a better idea.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license