New developer experience / on-boarding is a bit difficult

[lvaroqui]

Hello @sebadob,

As I've began to work on rauthy in the context of #872, I just wanted to share a bit of feedback on my dev experience so far.

DISCLAIMER: I don't want to be mean at all, I want to share info that could help make the on-boarding of new devs a better experience. To be honest my first few minutes with the project almost made me abandon the idea of contributing. This feedback is based on my own sensibility and I totally understand every one would not agree.

1. The CONTRIBUTING.md is missing the just setup step (which was in it before but was removed?).
2. Without a .env file with the DEV_MODE=true in it, the front-end will not actually build and this is not stated in CONTRIBUTING.md.
3. The generation of UI files that makes the git repo instantly dirty with ~700 changes is maybe the worst part:
   Committing generated artifact is commonly accepted as a bad practice. At least in general, the generated file are stable, meaning that if I do modify the UI, I shouldn't have modified files in my repo. Having to deal with such shenanigans at work, committed generated files are especially annoying when rebasing as every rebase must rebuild the files (because git does not understand how to solve the issue).
   I understand this is done to be able to build on FreeBSD but I feel like generating the files in CI in a Linux env and sending them to a FreeBSD env afterward is a much better solution than forcing developer to deal with committed generated files.
4. I don't like that even after stopping the just run command some docker container still run, I should not have any process remaining when I stop my dev env.
   For this, I really think that having a docker-compose flow would help immensly and would also avoid running just command to setup everything. The whole dev env could be embedded in a compose setup and a simple docker compose up would spin up everything.
   This would also avoid the "container already started issue".
5. The way hiqlite is integrated is a bit annoying to work with as all DB related stuff is duplicated. That's not too bad and understandable for things like migrations, but way harder to deal with when every access to the DB has to do twice the work with the infamous if is_hiqlite().

So these are my main complaints and I think points 1 through 4 could be resolved quite easily!

There also a lack of dev documentation (regarding architecture, db migration, ui internationalization, ui styling...) but that's understandable as this is mostly a one man project and I was able to get around quite easily in the end, so not that bad!

[sebadob]

Thanks for the feedback. Actually, I can only really improve all of this if I get feedback. To me, all of this is 2nd nature and it's super easy to forget something.

The CONTRIBUTING.md is missing the just setup step (which was in it before but was removed?).

That's actually a mistake and should be added back in. I removed a lot of tools from the setup step, which are not necessary anymore.

Without a .env file with the DEV_MODE=true in it, the front-end will not actually build and this is not stated in CONTRIBUTING.md.

I was not even aware of this. I have a frontend/.env with DEV_MODE=false in it, but this is in .gitignore. I will add it inline inside the frontend/package.json for the build script, which will fix this. Thanks!

The generation of UI files that makes the git repo instantly dirty with ~700 changes is maybe the worst part

Yeah I know about this and it's annoying me as well. Checking these files into version control was an easy fix, but I also thought about either adding a just recipe which can optionally build the UI inside some random npm container, or add a one-liner for this to the CONTRIBUTING.md. If I remember correctly, when I did this, running a container to build the FreeBSD package was not possible at that point. Otherwise I would have done this from the beginning. However, then the CI for FreeBSD needs a fix for this.

I don't like that even after stopping the just run command some docker container still run, I should not have any process remaining when I stop my dev env.

You don't stop your whole dev env with this. This would be just backend-stop. Starting the other containers each when time you did a small code change and need to recompile would make you lose your database for instance which is very annoying on another level, and also your emails you might want to have around from testing before. Starting and stopping would also take more time. This is why it's not built into the same command.
However, if someone likes this better, it could be easily done with an additional just recipe that takes care of the whole thing in a few lines.

The way hiqlite is integrated is a bit annoying to work with as all DB related stuff is duplicated.

Yeah, I tried my best to reduce the boilerplate down to a minimum with even pushing all Postgres queries into the DB::pg_* functions, but because of how very different query params are built, it's not possible to get rid of the rest, at least if you don't want to waste some memory.

There also a lack of dev documentation (regarding architecture, db migration, ui internationalization, ui styling...) but that's understandable as this is mostly a one man project and I was able to get around quite easily in the end, so not that bad!

That's something that can be added / updated. Most of the stuff is pretty straight forward, but having a few written words about it would help for sure.

---

Thanks for the feedback. As said already it's very hard to not forget about anything, if I don't get feedback from someone who has never touched the code before. So this really helps.

[sebadob]

Ahh yes, if you like cargo to automatically re-compile, then this makes a lot more sense. You could add it to the compose recipe. It would make a lot of sense there. Then maybe also another name than compose would be a good idea.

[lvaroqui]

I think watch could be nice?

[lvaroqui]

Another thing is that I did not actually realize that stop-backend removes the containers. Maybe that's a bit brutal? If I'm working on a feature during multiple days I may want to keep the content of my DB but not have it running constantly?

Would moving the removal to a dedicated rm-backend make sense? The only downside I can think of is if you do multiple clones (with git worktree for instance) you may end up with clashing containers. But that seems pretty minor as it's not very likely and easy to fix.

Also, I'm not sure backend is the most clear naming as I tend to think of the backend being the rauthy server (as opposed to the front-end which is Svelte UI). Maybe something like start-dev-env, stop-dev-env, rm-dev-env would have clearer intent?

[lvaroqui]

I will send a PR for the watch feature!

[sebadob]

Yes we could rename them, that's no issue at all of me.

Keep in mind that you only lose state, if you develop against Postgres. Hiqlite is persisted locally in data/. I usually only use Postgres if I am updating DB queries or inside integration tests of course. If you switch between branches, you could actually use the different databases to your advantage in that case and use Hiqlite for longer ongoing work and Postgres if you want to switch in between to do something else.

[lvaroqui]

Oh and maybe one last thing that could be added to the docs is how testing works. There's the Test Debugging section giving some hints but a couple of lines regarding where the test are located, how to write them could be nice. Also, it could be nice to explain how to test an implementation against an actual OIDC service, I believe for this case one should actually build a production version of rauthy and deploy it on an actual VPS to have a proper domain name.

[sebadob]

Ah yes, I could add this too. The integrations tests will probably be reworked a little bit to make compilation faster, so they end up in one module, which then calls all of them. The tests will be same same, just less compiled testing packages.

For the tests against real other APIs, I recently started doing that via the /dev/ endpoints and found that to be quite nice. It get's rid of any additional, external deployment and is no issue at all, since they are not compiled into release mode binaries. An example would be the test for backchannel logouts:

rauthy/src/api/src/dev_only/dev_handler.rs

Line 72 in 343e0aa

"backchannel_logout" => {

I will probably add SCIM tests that way as well.