nomad-nmr / nomad-server

Server side code for NOMAD system
https://www.nomad-nmr.uk/
GNU Affero General Public License v3.0
3 stars 2 forks source link

Make dev and prod routes consistent #142

Closed lukasturcani closed 1 month ago

lukasturcani commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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.