Skip to content

Add support for rebasing EOL extensions #49

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add support for rebasing EOL extensions #49

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b7cbe3f
jobs: Print the executed command
doraskayo Dec 17, 2021
b38d4cf
jobs: Consider every build ref when determining the EOL prefix
doraskayo Dec 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/jobs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ fn job_log_and_error(job_id: i32, conn: &PgConnection, output: &str) {

fn do_command(mut cmd: Command) -> JobResult<()>
{
info!("Running command: {:?}", &cmd);

let output =
unsafe {
cmd
Expand Down Expand Up @@ -359,6 +361,12 @@ impl CommitJobInstance {
}
}

fn get_main_build_ref_name (build_refs: &Vec<models::BuildRef>) -> &str {
Copy link
Member

Choose a reason for hiding this comment

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

Should be a slice instead of a &Vec no?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure, should it? What would be the benefit of using a slice in this case?

I'm definitely not an expert in Rust, so I'm open to any reasonable suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

See https://rust-lang.github.io/rust-clippy/stable/index.html#ptr_arg

Copy link
Member

Choose a reason for hiding this comment

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

A getter function shouldn't have a get prefix

Copy link
Author

@doraskayo doraskayo Dec 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It's not really a getter function, I guess. It's more of a helper function to find the main ref. Perhaps calling it find_main_build_ref_name would make it clearer?

return &build_refs.iter().min_by_key(|build_ref| {
build_ref.ref_name.split('/').nth(1).unwrap().len()
}).unwrap().ref_name
}

fn do_commit_build_refs (&self,
build_refs: &Vec<models::BuildRef>,
config: &Config,
Expand All @@ -373,11 +381,8 @@ impl CommitJobInstance {
let mut commits = HashMap::new();

let endoflife_rebase_arg = if let Some(endoflife_rebase) = &self.endoflife_rebase {
if let Some(app_ref) = build_refs.iter().filter(|app_ref| app_ref.ref_name.starts_with("app/")).nth(0) {
Some(format!("--end-of-life-rebase={}={}", app_ref.ref_name.split('/').nth(1).unwrap(), endoflife_rebase))
} else {
None
}
let old_prefix = CommitJobInstance::get_main_build_ref_name(build_refs).split('/').nth(1).unwrap();
Some(format!("--end-of-life-rebase={}={}", old_prefix, endoflife_rebase))
} else {
None
};
Expand Down