-
Notifications
You must be signed in to change notification settings - Fork 74
Adding the End to End flow #278
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
Conversation
| from flow.utils.paths import BenchmarkPaths, simpoint_analysis_root | ||
| from flow.utils.util import CommandError, Util | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed the TraceGenerator class definition was removed. Was there a specific reason for this change? The original idea behind the class was to make it easier to reuse in other scripts if needed.
| ) -> subprocess.CompletedProcess: | ||
| docker_cmd = self._docker_prefix(workdir, interactive) + ["bash", "-lc", shlex.join(command)] | ||
| Util.info("Docker exec: " + " ".join(docker_cmd)) | ||
| result = subprocess.run(docker_cmd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason the Docker Python library was removed? I found that using it made the code easier to read and maintain.
|
@Jatin-exe Please provide an ETA to address the review comments. |
Apologies for the delay. I will be pushing changes by EOD. Thank you for your patience. |
Hello @Jatin-exe do you have an update? When do you expect to push all the changes needed for the review? |
94fda6d to
6f49d5f
Compare
Implemented:
To do: