Add support for rebasing EOL extensions #49
Conversation
It's useful to be be able to see the executed command even when it's successful.
We assume that:
* There is always at least one build ref being committed.
* Committed ref names always include a ref type followed by a "/"
character. For example, "app/" or "runtime/".
* The main ref isn't necessarily an "app/" ref. For example, in the
case of extension-only repos, the main ref is a "runtime/" ref.
* The main ref being committed has the shortest ref name when
removing its ref type. For example:
* When the committed ref names are "app/org.app.App" and
"runtime/org.app.App.Plugin", the main ref is "app/org.app.App".
* When the committed ref names are "runtime/org.app.App.Plugin"
and "runtime/org.app.App.Plugin.Debug", the main ref is
"runtime/org.app.App.Plugin".
* The main ref with its ref type removed should be considered to be
the old prefix for the purpose of EOL rebasing.
This should allow rebasing EOL extensions in addition to apps.
Technically this assumption may be incorrect. As I understand, |
| @@ -359,6 +361,12 @@ impl CommitJobInstance { | |||
| } | |||
| } | |||
|
|
|||
| fn get_main_build_ref_name (build_refs: &Vec<models::BuildRef>) -> &str { | |||
There was a problem hiding this comment.
Should be a slice instead of a &Vec no?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
| @@ -359,6 +361,12 @@ impl CommitJobInstance { | |||
| } | |||
| } | |||
|
|
|||
| fn get_main_build_ref_name (build_refs: &Vec<models::BuildRef>) -> &str { | |||
There was a problem hiding this comment.
A getter function shouldn't have a get prefix
There was a problem hiding this comment.
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?
I couldn't find documentation which actually explains this aspect. Are you familiar with any repo where this funtionality is used? If I had access to one I could take a look and adjust the PR if necessary. |
Not with unrelated extension ID, but here is |
From the main commit message:
Note that while the assumptions above are based on my impression, they are not necessarily all correct. Please review thoroughly.