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

DRAFT: Add suggested fixes for known build issues #376

Open
wants to merge 1 commit into
base: branch-24.03
Choose a base branch
from

Conversation

jjwilke
Copy link

@jjwilke jjwilke commented Sep 21, 2022

No description provided.

@jjwilke jjwilke added the category:improvement PR introduces an improvement and will be classified as such in release notes label Sep 21, 2022
@jjwilke jjwilke requested a review from bryevdv September 22, 2022 01:01
@jjwilke jjwilke changed the title Add suggested fixes for known build issues DRAFT: Add suggested fixes for known build issues Sep 22, 2022
@jjwilke
Copy link
Author

jjwilke commented Sep 22, 2022

This needs to be changed to both capture output and continue to pipe output to the terminal. I don't recall an easy way to do this with Python.

OKGREEN = "\033[92m"
WARNING = "\033[93m"
FAIL = "\033[91m"
ENDC = "\033[0m"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could imitate the API I've used elsewhere for consistency

https://github.com/nv-legate/legate.core/blob/5a73122b7516f6b8e786d0c649522223aedc20fe/legate/driver/ui.py#L67-L68

(just manually, w/o using colorama)

@bryevdv
Copy link
Contributor

bryevdv commented Sep 22, 2022

@jjwilke it seems non-trivial enough that there is an entire package for doing it https://github.com/pycontribs/subprocess-tee

Maybe we could simplify it and vendor the parts we need. Otherwise, you can capture stdout and then just print it back out yourself, but obviously that won't update the console "in real time"

bryevdv
bryevdv previously approved these changes Sep 22, 2022
@manopapad manopapad changed the base branch from branch-22.10 to branch-22.12 September 30, 2022 21:59
@marcinz marcinz changed the base branch from branch-22.12 to branch-23.03 January 26, 2023 01:07
@marcinz marcinz dismissed bryevdv’s stale review January 26, 2023 01:07

The base branch was changed.

@marcinz marcinz changed the base branch from branch-23.03 to branch-23.05 March 6, 2023 22:20
@marcinz marcinz changed the base branch from branch-23.05 to branch-23.07 May 18, 2023 20:15
@marcinz marcinz changed the base branch from branch-23.07 to branch-23.09 July 18, 2023 15:40
@marcinz marcinz changed the base branch from branch-23.09 to branch-23.11 September 26, 2023 00:25
@marcinz marcinz changed the base branch from branch-23.11 to branch-24.01 November 9, 2023 16:54
@marcinz marcinz changed the base branch from branch-24.01 to branch-24.03 February 22, 2024 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:improvement PR introduces an improvement and will be classified as such in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants