Dockerize for GitHub execution #4
Conversation
|
Hi! Sorry about being a bad maintainer and not being responsive, I never saw that PR. I think dockerizing the tool is a good idea, so I'll be happy to work with you on getting this merged. You may want to start with rebasing your PR, I have just updated the dependencies. |
| @@ -67,7 +67,7 @@ pub fn parse_commits( | |||
| ) -> Result<Commits> { | |||
| lazy_static! { | |||
| static ref SHORTCUT_RE: Regex = | |||
| Regex::new(r"^\[(?:sc-|ch)(\d+)\]").expect("Could not compile SHORTCUT_RE"); | |||
| Regex::new(r"(?:\[sc-|\[ch|story/)(\d+)").expect("Could not compile SHORTCUT_RE"); | |||
There was a problem hiding this comment.
Is story a prefix supported by Shortcut for linking commits to issues automatically? The documentation does not suggest it...
If it's not the case, this would be better as a separate PR.
If the idea is to support user-defined prefixes, why not. But this means making it configurable in config.toml (an optional array would be ideal). We would also need either to restrict the prefixes to something like [a-zA-Z]+ (or be ready to escape characters.
One last point, your commit would also find a comment like Unlike what we did for [sc-XXX], this is actually a good idea. If the purpose is to find badly formatted commits like [sc-XXX] I suggest to simply trim the string/allow ^\s* at the start.
There was a problem hiding this comment.
I added story to the regex to cover missed associations but a story URL was in the PR history
Story details: https://app.shortcut.com/xyz/story/####
This was happening when a PR title did not reference the story ID, but a commit or the branch name linked to the story. Shortcut was pushing story link annotation onto the PR.
This change would detect those instances and make sure they were associated correctly.
The removal of ^ was to account for the URL case.
It also captured when the story id was not at the start of the commit message.
There was a problem hiding this comment.
All right, thanks. I'll have a look this week.
There was a problem hiding this comment.
All right, thanks. I'll have a look this week.
More like next week I'm afraid :(
|
@Emm I have rebased the branch. |
There was a problem hiding this comment.
I have started testing it, unfortunately the OpenAPI generator blows up with a stack overflow error. We may need to pin that to a specific version if possible.
There was a problem hiding this comment.
Can you post the error. I can look at it in a few days
This is not necessary since we use .gitattributes.
|
@jprosenbaum , I finally got around to working on this. Can you check out the
|
This is simpler and lets us control which version we get.
Loses the cache though.
Emm
force-pushed
the
dockerize-for-github-execution
branch
from
64f063a
to
0b7fc33
Compare
No description provided.