-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
port(Turborepo): Add github prefixing in Rust #6471
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
9 Ignored Deployments
|
🟢 Turbopack Benchmark CI successful 🟢Thanks |
🟢 CI successful 🟢Thanks |
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.
Approving since we're moving quick, but I do think we can keep the turborepo-ui
crate focused on just outputting stuff and not make it aware of CI vendors.
Very much looking forward to eventually getting #6314 merged in post-port.
crates/turborepo-ui/src/output.rs
Outdated
pub enum Fenceposts { | ||
Github(String), | ||
} | ||
|
||
impl Fenceposts { | ||
fn prefix_suffix(&self) -> (Option<String>, Option<String>) { | ||
match self { | ||
Self::Github(task_name) => ( | ||
Some(format!("::group::{task_name}\n")), | ||
Some("::endgroup::\n".to_string()), | ||
), | ||
} | ||
} | ||
} |
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.
This feels like this belongs in the turborepo-ci
crate instead of here.
pub fn as_github_task_id(&self) -> String { | ||
format!("{}:{}", self.package, self.task) | ||
} |
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.
Won't die on this hill, but this doesn't feel like it needs to be defined on TaskId
, I think it would better be defined as a function in turborepo-ci
maybe on the Vendor
type.
Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
Description
Slightly hacky way of doing this. It's a little expandable for more providers, but not all of the way. Getting all the way will require a larger output refactor.
Testing Instructions
Updated test for absolute paths in errors
Closes TURBO-1640