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

469 add initial cwd to session properties #472

Merged

Conversation

franekmagiera
Copy link
Contributor

Closes #469

Added a working directory nox was originaly invoked from to session propreties.

@DiddiLeija I started working on this issue some time ago, but then had a long break and couldn't find time to finalize it until today and now noticed that you already opened a PR for it. Hopefully we did not duplicate much work, sorry for not mentioning it earlier and please let me know if you are ok with me continuing on this one.

@DiddiLeija
Copy link
Collaborator

Oh, it's ok. I'll work on another issue. Go ahead!

@theacodes
Copy link
Collaborator

This looks great, and thanks for doing it @franekmagiera. :)

I don't love the name original_wd though. :/ I couldn't really find a lot of prior art for this term, but I think either session.executed_from or session.execution_dir might work a bit better. What do you think?

@henryiii
Copy link
Collaborator

henryiii commented Sep 9, 2021

To me, session.execution_dir sounds like the place it is executing, not the place it was called from. How about session.invocation_dir? I like it ending in dir vs. wd.

@franekmagiera
Copy link
Contributor Author

@theacodes Thanks!

Now that you mention it I agree original_wd does not make much sense without the context of #469.

I like your propositions, having the name end with dir is better I think. How about just session.starting_dir? It's a bit simpler, but I'm fine with session.invocation_dir as well.

@theacodes
Copy link
Collaborator

theacodes commented Sep 9, 2021 via email

@FollowTheProcess
Copy link
Collaborator

How about invoked_from? I know it doesn't specify dir explicitly but I think it's nice and clear, especially when in the context of session.invoked_from

@theacodes
Copy link
Collaborator

theacodes commented Sep 9, 2021 via email

@henryiii
Copy link
Collaborator

henryiii commented Sep 9, 2021

I'm okay with that too. It's a tossup for me, I like that the type is in the name with invocation_dir, but invoked_from probably can't be viewed as anything but a directory, and is a bit easier to read.

@franekmagiera
Copy link
Contributor Author

Ok, session.invoked_from it is then! Thanks for the discussion, I will make necessary changes tomorrow or on Saturday

@franekmagiera
Copy link
Contributor Author

@theacodes @henryiii @FollowTheProcess Changed the name to invoked_from and merged with main.

@FollowTheProcess
Copy link
Collaborator

Looks good to me! happy to merge in unless anyone has any other suggestions/comments? @theacodes @cjolowicz

@theacodes
Copy link
Collaborator

I have one plumbing change I want to make here. Let me do that and I'll merge. :)

@theacodes theacodes merged commit 74f8793 into wntrblm:main Sep 11, 2021
@theacodes
Copy link
Collaborator

Thanks, everyone!

@franekmagiera franekmagiera deleted the 469-add-initial-cwd-to-session-properties branch September 11, 2021 21:05
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.

Working directory unexpectedly set to wherever noxfile.py is.
5 participants