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

Couple suggestions #15

Open
szabolcsmaj opened this issue Jan 8, 2024 · 4 comments
Open

Couple suggestions #15

szabolcsmaj opened this issue Jan 8, 2024 · 4 comments
Assignees

Comments

@szabolcsmaj
Copy link

I wrote down a couple of things that can be changed/mentioned. These are just suggestions:

  • I'm a big fan of pinning dependencies in requirements.txt. It's a ticking timebomb if they are defined without any version. The downside is of course updating them later.
  • In the readme, you can shorted the shell execution into 1 line: docker-compose exec odoo sh -c "odoo shell --http-port=8071"
  • It might make sense to set the environment ODOO_STAGE too. This is the variable that odoo.sh uses to determine if the environment is dev/test, staging or production. If we are developing a module that uses this locally, it would be good to already have that set.
@yhaelopez
Copy link
Contributor

yhaelopez commented Jan 8, 2024

@szabolcsmaj Thank you for your comments! They are well received and appreciated!

For 1:
I didn't consider adding the version to every package in the requirements.txt unless any package causes any ussue (for example, the Sentry package we use).

However, I agree that as well as with containers, these packages should have the version tag even if it's the latest.

For 2:
Your suggestion in the README is perfect!
Also, this file is quite outdated.
I'll be updating it and move some stuff to the Repo's Wiki.

For 3:
Does ODOO_STAGE has any effect or would be just informative?

@szabolcsmaj
Copy link
Author

3: It would be just for convenience. For example, if I'd develop an application that changes its behavior based on the environment and I plan to develop it to odoo.sh, I can test it locally to see it behaves properly. I mean users can add it manually if they want to without too much effort. It's just an idea :)

@yhaelopez
Copy link
Contributor

@szabolcsmaj The requirements.txt is now version-fixed! 🎉

@yhaelopez
Copy link
Contributor

For number 2. I'm posting a new documentation by the end of this week.
And number 3: can we use our current APP_ENV for the purpose you mentioned or do you think it has a different purpose? APP_ENV can be used for both: informative & behavioral change.

@yhaelopez yhaelopez self-assigned this Jul 16, 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

No branches or pull requests

2 participants