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

Change activation call to be a list of commands #2424

Closed
wants to merge 3 commits into from

Conversation

kubouch
Copy link
Contributor

@kubouch kubouch commented Sep 25, 2022

ActivationTester will use a list of strings as the command instead of just a string.

Required to proceed with #2422

Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)!

  • ran the linter to address style issues (tox -e fix_lint)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

@kubouch kubouch force-pushed the activate-cmd-list branch 2 times, most recently from 04374e9 to 4f20993 Compare September 25, 2022 15:28
@kubouch
Copy link
Contributor Author

kubouch commented Sep 25, 2022

@gaborbernat Is something like this what you had in mind?

@kubouch
Copy link
Contributor Author

kubouch commented Oct 15, 2022

@gaborbernat Could you please comment whether this approach is what you had in mind when making the activation command a list of strings? Should I continue debugging it to make it work on Windows or is it something else you meant? Reference conversation: #2422 (comment)

@gaborbernat
Copy link
Contributor

Yeah you are on the right path.

@gaborbernat
Copy link
Contributor

@kubouch updates on this?

@kubouch
Copy link
Contributor Author

kubouch commented Oct 25, 2022

I didn't have time to investigate the Windows issue because I had to focus on my actual work but I'd like this to be finished. If you have an idea why it's failing on Windows, that would help a lot.

@gaborbernat
Copy link
Contributor

Not sure sadly.

@gaborbernat
Copy link
Contributor

Seems no longer needed.

@kubouch
Copy link
Contributor Author

kubouch commented Nov 27, 2022

OK, so the activation command being a list is no longer necessary?

@gaborbernat
Copy link
Contributor

Would still be nice to make the change, but seems we can't get it working 👍

@kubouch
Copy link
Contributor Author

kubouch commented Nov 27, 2022

Yeah, it's something related to the Windows paths but I couldn't locate the issue.

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.

None yet

2 participants