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

AWS Paas Migration Changes #1264

Merged
merged 3 commits into from
Jul 19, 2024
Merged

AWS Paas Migration Changes #1264

merged 3 commits into from
Jul 19, 2024

Conversation

PaulWheatcroft
Copy link
Contributor

Changes to enable AWS migration

Why

To facilitate the application(s) in the repo to be deployed on to the new AWS platform

@paulpepper-trade paulpepper-trade marked this pull request as ready for review July 16, 2024 13:53
@paulpepper-trade paulpepper-trade requested a review from a team as a code owner July 16, 2024 13:53
package.json Outdated
@@ -44,6 +44,7 @@
"clean": "rm -f ./run/static/webpack_bundles/*",
"heroku-prebuild": "",
"heroku-postbuild": "npm run build",
"start": "echo starting",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is start a placeholder for a future script? (We've tended not to introduce placeholders into production code.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again this is a work-around for node.js buildpack

Copy link
Collaborator

@paulpepper-trade paulpepper-trade Jul 18, 2024

Choose a reason for hiding this comment

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

Agreed with @emileswarts to replace "echo starting" with something that documents the dummy "start" script entry - e.g. "start": "dummy entry for node.js buildpack workaround".

Copy link
Contributor

Choose a reason for hiding this comment

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

Pointing this at bash web-worker-entrypoint.sh will be sufficient for this workaround, but means that we can't add the comment to indicate that it's a workaround.

@PaulWheatcroft PaulWheatcroft marked this pull request as draft July 17, 2024 08:27
@emileswarts emileswarts marked this pull request as ready for review July 18, 2024 14:24
@emileswarts emileswarts force-pushed the paas-migration branch 3 times, most recently from d30c74f to 805ca50 Compare July 19, 2024 10:07
Copy link
Collaborator

@paulpepper-trade paulpepper-trade left a comment

Choose a reason for hiding this comment

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

LGTM

@emileswarts emileswarts merged commit 3cf4f34 into master Jul 19, 2024
9 checks passed
@emileswarts emileswarts deleted the paas-migration branch July 19, 2024 14:20
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.

3 participants