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

ELEC-627: Update codegen-tooling setup instructions #37

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

andyjiin
Copy link
Member

@andyjiin andyjiin commented May 16, 2019

In preparation for new member on-boarding:

  • Clarify setup instructions for codegen-tooling
  • We are transitioning to DBC so we want them to be able to create a new CAN message and generate the DBC accordingly

@karlding
Copy link
Contributor

I question whether this is the correct approach (ie. encouraging people to install libraries globally), as opposed to updating the Vagrant box to choose 1 of {pyenv, virtualenv, virtualenvwrapper, pipenv, conda, et al.} and blessing that by including it by default as the supported configuration. I'm curious why this approach was chosen?

Generally each project should manage their own set of dependencies separately, otherwise you run into issues with resolving package versions and constantly having to update dependencies in lockstep. Currently requirements.txt freezes the current version of requirements (as it should), and so if two projects need the same library (but happen to be using different versions), they will conflict unless one is bumped.

If it's a package that is to be made available on the shell, then perhaps it would make sense to install globally. But project dependencies imo should always be local.

@aaronhktan
Copy link
Member

I agree with @karlding; using a virtual environment in Python is a good practice with practical benefits as Karl mentioned above.

DBC changes look good though!

@andyjiin
Copy link
Member Author

andyjiin commented May 16, 2019

@karlding @aaronhktan Sure noted, I will leave it as is. One thing I noticed was that the virtualenv .venv command wasn't working on most Windows machines unless environment variable VIRTUALENV_ALWAYS_COPY=1. Is this a known issue?

@karlding
Copy link
Contributor

That's probably more to do with the fact that we're exposing a shared folder from a Windows Host to a Linux Guest, since virtualenv is trying to create symlinks (and not sure how that'll work if you have symlinks created on Linux that also need to appear in Windows, which sounds like a nightmare when the Windows equivalent is hard links/junctions).

You can try playing around with the mount options in the Vagrantfile on the shared directory to see if that fixes it, but I think that's just a limitation of cloning things in the shared folder. If you clone into a folder that isn't mounted (ie. something not under shared/), then it shouldn't be necessary.

Otherwise, you can also add host OS specific initialization in the Vagrantfile to export that variable.

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.

3 participants