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

Make dev and prod routes consistent #142

Merged
merged 8 commits into from
Oct 24, 2024

Conversation

lukasturcani
Copy link
Contributor

@lukasturcani lukasturcani commented Oct 7, 2024

Small PR to keep routes consistent between dev and prod environments.

The change rests on two main things

  1. backend routes are explicitly prefixed with /api. I think this makes more sense anyway, when someone reads nomad-rest-api/app.js it is immediately clear what the correct routes are. As it currently stands the reader also has to have knowledge about how the app is deployed with nginx to know that the routes would need to be prefixed with /api/ if they wish to send requests to the backend.
  2. stop nginx from stripping /api/ from routes forwarded to the server. Because the backend explicitly serves routes prefixed with /api/ this bit of config is no longer necessary.

Small optional changes:

  1. removed version in docker-compose.yaml - my docker compose keeps complaining that this key is not longer supported
  2. Tests now run when a new commit is pushed to the PR. currently if someone submits a new commit to the PR the tests do not run again which means that issues are no longer reported.

@lukasturcani
Copy link
Contributor Author

lukasturcani commented Oct 7, 2024

also -- its probably worth only merging PRs with squash and merge -- so that the history of the main branch does not get contaminated with WIP commits. the other kinds of merge can be disabled in the settings for the repo

@tomlebl
Copy link
Contributor

tomlebl commented Oct 24, 2024

@lukasturcani I have finally got time to have a look on this.
It looks that you took the laborious route that I did not dare or was too lazy to take.
"Squash & Merge" makes sense but can't find where to disable the other commits.

@tomlebl tomlebl merged commit b8dc208 into nomad-nmr:main Oct 24, 2024
2 checks passed
@lukasturcani lukasturcani deleted the lukas/consistent-routes branch October 24, 2024 22:33
@tomlebl tomlebl mentioned this pull request Oct 25, 2024
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.

2 participants