-
Notifications
You must be signed in to change notification settings - Fork 4
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
Improving version v.23.08.03 #5
base: master
Are you sure you want to change the base?
Conversation
/usr/bin/git clone -b master $LIBRARIES_REPOSITORIES $ODOO_WORK_DIR/waftlib | ||
/bin/echo "( INFO ) : Remove exist ${ODOO_WORK_DIR}/.ignore/waftlib" | ||
/bin/rm -fr "${ODOO_WORK_DIR}/.ignore/waftlib" && \ | ||
/bin/mkdir -p "${ODOO_WORK_DIR}/.ignore/waftlib" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will need time for a full review, but one thing I have doubts about is the .ignore
directory, I understand that contains all the stuff that git should not add to the repository, but having a few more entries in .gitignore seems to me a very minor problem.
I think it might help in understanding the structure if we follow the conventions used in a lot of projects. Like putting libraries needed before anything in a lib directory (./lib/waftlib
), putting every artefact created by build in a ./build
directory.
Maybe not crucial for how it all works, but when we are moving and reorganizing directories anyway, why not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will need time for a full review,
take your time.
but one thing I have doubts about is the .ignore directory, I understand that contains all the stuff that git should not add to the repository, but having a few more entries in .gitignore seems to me a very minor problem.
in my opinion, just the all stuff in .ignore
directory that git should not add to the repository, not .ignore
directory contains all the stuff that git should not add to the repository.
I think it might help in understanding the structure if we follow the conventions used in a lot of projects. Like putting libraries needed before anything in a lib directory (./lib/waftlib), putting every artefact created by build in a ./build directory.
in my opinion, we don't have any library in this code that we import from outside, this project is not a python code, it is a few scripts (sometimes python scripts, sometimes bash or shell scripts), and 'lib' directories confused others, we have a small code that we use it in many scripts so we putted it in /.venv-*/lib/python*/site-packages/venv*lib/
as link, and I didn't see it is a reason to have a directory for it in the main project directory.
Maybe not crucial for how it all works, but when we are moving and reorganizing directories anyway, why not?
both are good for me, just I did what I liked, we can discuss it more with other members in our team.
@ all
This v.23.08.03 demo version, I closed sunflowerit/waftlib#34 and create this new one, any new idea will be welcome.
Don't merge this branch to master, still need more work.