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

Save relative paths from the config module #5364

Closed
stsewd opened this issue Feb 27, 2019 · 2 comments · Fixed by #5377
Closed

Save relative paths from the config module #5364

stsewd opened this issue Feb 27, 2019 · 2 comments · Fixed by #5377
Assignees
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code

Comments

@stsewd
Copy link
Member

stsewd commented Feb 27, 2019

We currently are saving the whole path, including the root of the servers, that info isn't useful for us or for the user later. We will not lose any kind of information, because we already have the base path to validate files. This shouldn't be a big refactor, and we have a lot of tests.

Ref #4863 (comment)

@stsewd stsewd added Improvement Minor improvement to code Accepted Accepted issue on our roadmap labels Feb 27, 2019
@stsewd stsewd self-assigned this Feb 27, 2019
@humitos
Copy link
Member

humitos commented Feb 27, 2019

I saw that we are using absolute path in other places, not just in this config module. For example, on the build output page --which generate to scroll horizontally to know the command executed.

Do you think it worth to consider to user relative path in other places also? What are your thoughts on this?

@stsewd
Copy link
Member Author

stsewd commented Feb 27, 2019

Do you think it worth to consider to user relative path in other places also? What are your thoughts on this?

Yeah, when I refactored the code, I tried to put most of the commands with relative paths (like requirements, conda env, etc).

We still depend on absolute paths to set the cache to pip, virtualenv root, python binnary.

We can remove some of those using env variables, after all, isn't helpfull at all for the user to know where we are saving the cache/virtualenv. This is another issue, not in the scope of this one.

stsewd added a commit to stsewd/readthedocs.org that referenced this issue Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants