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.

Sign up for GitHub

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

Jump to bottom

RQD: fixes for runWindows #1215

Merged
merged 2 commits into from
Oct 27, 2022
Merged

RQD: fixes for runWindows #1215

merged 2 commits into from
Oct 27, 2022

Conversation

ndeebook
Copy link
Contributor

Hello !

Here are the changes I needed to make RQD work for our usage on Windows.
Let me know if I need to change anything.

RQD Fixes for Windows:

  • Use environment with subprocess.Popen on Windows (runWindows)
  • Fix environment for Windows (add PATH, remove MAIL and HOME)
  • Escape some characters in the .bat file

RQD Fix for all platforms:

  • Don't replace the PATH variable but extend it instead (it was required for some of my usage on Windows)

Issues:

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 18, 2022 •
edited

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: ndeebook / name: Olivier Evers (6b6c3c2)

@ndeebook ndeebook force-pushed the rqd_bugfix_use-env-on-windows branch from 272376d to 9b6bbda Compare October 18, 2022 08:39
@ndeebook
RQD: fixes for runWindows
6b6c3c2 
* fix and use environment in subprocess.Popen
* escape some characters in the .bat file
@ndeebook ndeebook force-pushed the rqd_bugfix_use-env-on-windows branch from 9b6bbda to 6b6c3c2 Compare October 18, 2022 11:21
DiegoTavares
DiegoTavares approved these changes Oct 26, 2022
View reviewed changes
Copy link
Collaborator

@DiegoTavares DiegoTavares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@bcipriano bcipriano merged commit 3742ff0 into AcademySoftwareFoundation:master Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DiegoTavares DiegoTavares DiegoTavares approved these changes

@bcipriano bcipriano bcipriano approved these changes

@gregdenton gregdenton Awaiting requested review from gregdenton

@jrray jrray Awaiting requested review from jrray

@smith1511 smith1511 Awaiting requested review from smith1511

@larsbijl larsbijl Awaiting requested review from larsbijl

@IdrisMiles IdrisMiles Awaiting requested review from IdrisMiles

@splhack splhack Awaiting requested review from splhack

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ndeebook @DiegoTavares @bcipriano