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

#556: update PATH on pc run #614

Merged
merged 11 commits into from
Sep 19, 2024
Merged

#556: update PATH on pc run #614

merged 11 commits into from
Sep 19, 2024

Conversation

hohwille
Copy link
Member

@hohwille hohwille commented Sep 12, 2024 •
edited
Loading

fixes #556

  • add withPathEntry(Path) method for future implementations of setEnvironment methods
  • add ability to create a modified SystemPath via withPath method
  • change ProcessContext to track PATH changes and apply them only in run method
  • also move the code to find the binary executable to run method using the final PATH
  • improved logging
  • changed ToolCommandlet to simplify after this "bug" has been fixed

@hohwille hohwille added bugfix environment EnvironmentCommandlet, env variables, path, etc. process executing external programs (ProcessContext) labels Sep 12, 2024
@hohwille hohwille added this to the release:2024.09.002 milestone Sep 12, 2024
@coveralls
Copy link
Collaborator

coveralls commented Sep 12, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 10909858158

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 67 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.1%) to 65.632%
Files with Coverage Reduction New Missed Lines %
com/devonfw/tools/ide/process/ProcessContext.java 1 31.58%
com/devonfw/tools/ide/context/IdeContext.java 2 58.75%
com/devonfw/tools/ide/log/IdeLogger.java 2 63.04%
com/devonfw/tools/ide/tool/ToolCommandlet.java 13 76.88%
com/devonfw/tools/ide/common/SystemPath.java 20 76.83%
com/devonfw/tools/ide/process/ProcessContextImpl.java 29 76.81%
Totals Coverage Status
Change from base Build 10907807987: -0.1%
Covered Lines: 6012
Relevant Lines: 8817
💛 - Coveralls

jan-vcapgemini
jan-vcapgemini requested changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@jan-vcapgemini jan-vcapgemini left a comment

Choose a reason for hiding this comment

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

LGTM. Added some small CRs.

cli/src/main/java/com/devonfw/tools/ide/common/SystemPath.java Show resolved Hide resolved
cli/src/main/java/com/devonfw/tools/ide/common/SystemPath.java Show resolved Hide resolved
cli/src/main/java/com/devonfw/tools/ide/common/SystemPath.java Show resolved Hide resolved
jan-vcapgemini
jan-vcapgemini approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@jan-vcapgemini jan-vcapgemini left a comment

Choose a reason for hiding this comment

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

Ready to merge.

@hohwille hohwille merged commit 74b4cc7 into devonfw:main Sep 19, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jan-vcapgemini jan-vcapgemini jan-vcapgemini approved these changes

Assignees

@hohwille hohwille

Labels
bugfix environment EnvironmentCommandlet, env variables, path, etc. process executing external programs (ProcessContext)
Projects
IDEasy board
Status: ✅ Done
Milestone
release:2024.09.002
Development

Successfully merging this pull request may close these issues.

ProcessContext should compute PATH on run and not in constructor
3 participants
@hohwille @coveralls @jan-vcapgemini