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

I fixed a typo in EnvSearchServiceInternalPort that was EnvSrchServiceInternalPort #503

Closed
wants to merge 3 commits into from

Conversation

AmineAouragh
Copy link

@AmineAouragh AmineAouragh commented Oct 17, 2023

Issue reference: #492

There are typos in aws/cloudformation-templates/services/service/_template.yaml where the word 'Serch' should be 'Search' in lines 154 and 305

I fixed them here
2023-10-18 13_34_37-I fixed a typo in EnvSearchServiceInternalPort that was EnvSrchServiceInternalPo

and here
2023-10-18 13_38_05-I fixed a typo in EnvSearchServiceInternalPort that was EnvSrchServiceInternalPo

@BastLeblanc
Copy link
Contributor

Hi, thanks for your pull request, before accepting it, can you check these:

  • if it's solving an issue can you reference it and describe it. Or explain that it's just cosmetic on a typo.
  • have you tried deploying the project with your changes? It seems that you haven't modified the name in all the right places.
  • ensure the integration tests are all passing (see https://github.com/aws-samples/retail-demo-store/tree/master/src/run-tests for how to run the integration tests)

Looking forward for your answers

@AmineAouragh
Copy link
Author

Hey Bastien! Thanks for the reply. I will make sure of that and fix everything I missed regarding that issue. I think I'll need to add more details in the pull request description. Thank you.

@AmineAouragh
Copy link
Author

AmineAouragh commented Oct 18, 2023

I just fixed the same typo - 'Serch' should be 'Search' as mentioned in the issue #492 - in 3 different locations in the _template.yaml file here -> retail-demo-store/aws/cloudformation-templates/services/service/_template.yaml

Here:
2023-10-18 13_34_37-I fixed a typo in EnvSearchServiceInternalPort that was EnvSrchServiceInternalPo

and here:
2023-10-18 13_38_05-I fixed a typo in EnvSearchServiceInternalPort that was EnvSrchServiceInternalPo

@AmineAouragh
Copy link
Author

More details were added to the pull request description

@BastLeblanc BastLeblanc self-assigned this Oct 18, 2023
@BastLeblanc
Copy link
Contributor

@AmineAouragh the deployment fails after your proposed PR with the error for each service:
Parameters: [EnvSearchServiceInternalPort] do not exist in the template

Please do a full end to end testing of deploying the cloudformation template before proposing a PR. thanks in advance.

@BastLeblanc
Copy link
Contributor

I'm closing this PR, please reopen if you get to run the tests before proposing the PR. Thanks

@BastLeblanc BastLeblanc closed this Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants