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

Address #319: Give a create_tmp API #320

Merged
merged 6 commits into from May 16, 2020

Conversation

moshez
Copy link
Contributor

@moshez moshez commented May 3, 2020

This doesn't have docs or tests! Hooray :)

It does however show one way to implement it (kinda hacky) and a working use case for further discussion.

@moshez
Copy link
Contributor Author

moshez commented May 3, 2020

Hi @theacodes ! This is a rough draft, but I want some feedback on how much in the wrong direction am I going. Is it reasonable to assume the bin directory is writable? Should we push this API to the specific virtualenv? Is it OK to fail if bin is None? Etc.

@theacodes
Copy link
Collaborator

This looks like a great start.

Is it reasonable to assume the bin directory is writable?

Yup.

Should we push this API to the specific virtualenv?

Nope, I think even "virtualenvless" sessions will have use cases for the tmpdir.

Is it OK to fail if bin is None?

I think this should work even if there's no virtualenv for the session.

@moshez moshez marked this pull request as ready for review May 7, 2020 02:59
@moshez
Copy link
Contributor Author

moshez commented May 7, 2020

Hi @theacodes ! I added tests and documentation. I'm not sure if I understood your response to the third question -- do you mean I should have a tmpdir even without bin? In that case, I'm not sure the location would still make sense.

@theacodes
Copy link
Collaborator

do you mean I should have a tmpdir even without bin? In that case, I'm not sure the location would still make sense.

Yup.

I think it makes sense for it to go in .nox/{session name}/tmp.

@moshez
Copy link
Contributor Author

moshez commented May 16, 2020

OK, let me know what you think now. It should always work now.

@theacodes
Copy link
Collaborator

Looks good. 👌

@theacodes theacodes merged commit 9110f3e into wntrblm:master May 16, 2020
@moshez moshez deleted the 319-temporary-directory branch May 16, 2020 14:48
@dhermes
Copy link
Collaborator

dhermes commented May 17, 2020

@moshez I'd love to chat about this feature and how it relates to my use / abuse of .nox/libbezier-debug and .nox/libbezier-release to cache and re-use debug and release builds of libbezier to speed up local dev for the Python parts of bezier: https://github.com/dhermes/bezier

@moshez
Copy link
Contributor Author

moshez commented May 31, 2020

@dhermes sent you an e-mail re: chatting about it. let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants