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

Update Readme and Refactor Pipeline Code #157

Merged
merged 6 commits into from
Mar 18, 2024

Conversation

williamputraintan
Copy link
Member

@williamputraintan williamputraintan commented Mar 15, 2024

  • Shorten yarn cdk syntax from yarn cdk-stateless-pipeline -> yarn cdk-stateless, the pipeline should just be the stack
  • make suite -> make test-suite which only test microservice tests
  • delete bin/orcabus.ts , alternative to deploy directly from local is from yarn cdk-stateless deploy {theBetaStack}
  • Updated docs (Happy for any feedback here!)
    • Update Readme.md
    • Update the MICROSERVICE.md
    • Added SHARED_RESOURCE.md to describe what resources is being shared within the stateful stack

Fix #150

Could be merged after #155

@williamputraintan williamputraintan added documentation Improvements or additions to documentation pipeline Workflow/Pipeline Manager labels Mar 15, 2024
@williamputraintan williamputraintan self-assigned this Mar 15, 2024
@williamputraintan
Copy link
Member Author

Did mention to switch the Parent of the microservice stack to a regular class to give a shorter stack name

From : OrcaBusStatelessPipeline/BetaDeployment/OrcaBusStatelessStack/PostgresManager
To: OrcaBusStatelessPipeline/BetaDeployment/PostgresManager

But in order to do that the class should be free from any resource lookup which we are using it here for shared resources. Maybe we could delay this for now?

@victorskl
Copy link
Member

RE: switch to a regular class & stack name long

Maybe we could delay this for now?

Yes, pls.

This is fine at this point. Either way, it only reduces one step in the Stack path. And we will be doing cdk list then copy & pasting it from there, anyway. So, optimising it at this point might turn into premature attempt. I'd vote, pls leave it for now and, let us tackle on other priority.

@victorskl
Copy link
Member

I will review the PR on Monday, eta.

README.md Show resolved Hide resolved
Copy link
Member

@victorskl victorskl left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Will.!

@williamputraintan
Copy link
Member Author

This will PR will require to cdk deploy the pipeline as the makefile command will change. Will merge this after #155

@victorskl
Copy link
Member

Got it, merging the other one.

resolution: "aws-cdk-lib@npm:2.132.1"
"aws-cdk-lib@npm:^2.133.0":
version: 2.133.0
resolution: "aws-cdk-lib@npm:2.133.0"
Copy link
Member Author

@williamputraintan williamputraintan Mar 18, 2024

Choose a reason for hiding this comment

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

@williamputraintan
Copy link
Member Author

cc @alexiswl @brainstorm for the change that shorten the yarn-cdk command

  • yarn cdk-stateful-pipeline -> yarn cdk-stateful
  • yarn cdk-stateless-pipeline -> yarn cdk-stateless

@williamputraintan williamputraintan merged commit 6d683c3 into main Mar 18, 2024
2 checks passed
@williamputraintan williamputraintan deleted the feature/pipeline-chores branch March 18, 2024 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation pipeline Workflow/Pipeline Manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Platform CodePipeline chores
3 participants