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

Make install_files invoked automatically for Driver object startup #1099

Merged
merged 3 commits into from
Jun 12, 2024

Conversation

rnemes
Copy link
Contributor

@rnemes rnemes commented Jun 6, 2024

Bug / Requirement Description

Clearly and concisely describe the problem.

Solution description

Describe your code changes in detail for reviewers.

Checklist:

  • Test
  • Example (both test_plan.py and .rst)
  • Documentation (API)
  • News fragment present for release notes
  • MS info leakage check
  • For new driver: driver index page
  • For new assertion: ui/pdf/std renderers, documentation
  • For new cmdline arg: documentation

@rnemes rnemes requested a review from a team as a code owner June 6, 2024 08:32
makedirs(directory)
self._binpath = bin_dir
self._etcpath = etc_dir
self.std = StdFiles(self.app_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Std files are not dirs, it might could go back to pre_start

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is true, but we have a circular dependency here, because we need self.std to be defined for install_files() to work properly and also cannot be executed before make_runpath_dirs() as it needs the directories being created here.
So another solution would be to rename make_runpath_dirs function to a better describing name, but does it worth the hassle?

@rnemes rnemes merged commit 5132c85 into morganstanley:main Jun 12, 2024
15 checks passed
@rnemes rnemes deleted the install_files branch June 12, 2024 13:28
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.

2 participants