Skip to content

pkg/oc/cli/admin/release: Support 'Bug X, Y, Z: ...' pull-request subjects #22225

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

Jump to bottom
Closed
wking wants to merge 1 commit into from
Closed

pkg/oc/cli/admin/release: Support 'Bug X, Y, Z: ...' pull-request subjects #22225

wking wants to merge 1 commit into from

Conversation

@wking
Copy link
Member

@wking wking commented Mar 4, 2019

For pulls that fix multiple bugs.

The rename from MergeCommit -> commit is because:

  • There is no need for a public structure that is part of this package's API, and
  • Consumers don't need to care if the commit is a merge or not.

The new issue structure sets the stage for future work to also support references to GitHub and other issue stores, although I haven't added extractors for those yet. At the moment, it just provides separation between the issue-extracting logic (which knows Bug ### means "Red Hat's Bugzilla") and the formatting logic (which just needs to know "there's an issue with a Markdown link").

CC @smarterclayton, @ironcladlou

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 4, 2019
@wking wking force-pushed the bug-x-y-z branch 2 times, most recently from 3e94fee to b14604e Compare March 4, 2019 20:14
@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 4, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wking
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: enj

If they are not already assigned, you can assign the PR to them by writing /assign @enj in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wking
pkg/oc/cli/admin/release: Support 'Bug X, Y, Z: ...' pull-request sub…
1a3dbbd 
…jects

For pulls that fix multiple bugs.

The rename from MergeCommit -> commit is because:

* There is no need for a public structure that is part of this
  package's API, and
* Consumers don't need to care if the commit is a merge or not.

The new 'issue' structure sets the stage for future work to also
support references to GitHub and other issue stores, although I
haven't added extractors for those yet.  At the moment, it just
provides separation between the issue-extracting logic (which knows
"Bug ###" means "Red Hat's Bugzilla") and the formatting logic (which
just needs to know "there's an issue with a Markdown link").
@wking wking mentioned this pull request Mar 4, 2019
Closed
@wking
Copy link
Member Author

wking commented Mar 4, 2019

images:

2019/03/04 20:49:20 error: unable to signal to artifacts container to terminate in pod release-latest, triggering deletion: could not run remote command: Internal error occurred: error executing command in container: unable to upgrade to tcp, received 500
2019/03/04 20:49:26 Ran for 33m35s
error: could not run steps: test "release-latest" failed: pod release-latest was already deleted

/retest

@wking
Copy link
Member Author

wking commented Mar 4, 2019

Hmm. Still no new jobs? Try explicit per-test commands:

/test images
/test e2e-aws
/test e2e-aws-serial

@smarterclayton
Copy link
Contributor

smarterclayton commented Mar 4, 2019

/refresh

@smarterclayton
Copy link
Contributor

smarterclayton commented Mar 4, 2019

It ran serial at 2019-03-04 16:52 EST

abhinavdahiya
abhinavdahiya reviewed Mar 5, 2019
View reviewed changes
pkg/oc/cli/admin/release/git.go
ParentCommits []string
// Local Merkle-graph node information
Subject string
CommitterDate time.Time
Copy link
Contributor

@abhinavdahiya abhinavdahiya Mar 5, 2019

Choose a reason for hiding this comment

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

CommitDate vs CommiterDate ?

Copy link
Member Author

@wking wking Mar 5, 2019

Choose a reason for hiding this comment

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

CommitDate vs CommiterDate ?

I'm fine leaving that off, but as long as I was touching this struct I thought I'd align with Git's usage (which is not perfectly consistent either):

$ git clone git://github.com/git/git.git
$ cd git
$ git grep -i committer.date v2.21.0 | wc -l
330
$ git grep -i commit.date v2.21.0 | wc -l
79
$ git grep -i committerdate v2.21.0 | wc -l
14
$ git grep -i commitdate v2.21.0 | wc -l
2

That said, CommitDate is the default in pretty logs (although not always a winner in the pretty docs :p):

$ git grep -i 'commit.*date' Documentation/pretty-formats.txt | cat
Documentation/pretty-formats.txt:	  CommitDate: <committer date>
Documentation/pretty-formats.txt:- '%cd': committer date (format respects --date= option)
Documentation/pretty-formats.txt:- '%cD': committer date, RFC2822 style
Documentation/pretty-formats.txt:- '%cr': committer date, relative
Documentation/pretty-formats.txt:- '%ct': committer date, UNIX timestamp
Documentation/pretty-formats.txt:- '%ci': committer date, ISO 8601-like format
Documentation/pretty-formats.txt:- '%cI': committer date, strict ISO 8601 format

So I'm leaning weakly towards CommitterDate but could be swayed ;).

@wking
Copy link
Member Author

wking commented Mar 5, 2019

Kicking again; maybe this time Prow will notice.

/test images
/test e2e-aws

@wking
Copy link
Member Author

wking commented Mar 5, 2019

Heh, seems like Prow is only picking up the last command in each comment? Just in case, here's a single comand to retry images:

/test images

@wking
Copy link
Member Author

wking commented Mar 5, 2019

All green :)

@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 13, 2019

@wking: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 13, 2019
@openshift-bot
Copy link
Contributor

openshift-bot commented Jun 11, 2019

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 11, 2019
@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 11, 2019

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 11, 2019
@openshift-bot
Copy link
Contributor

openshift-bot commented Aug 10, 2019

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 10, 2019

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@enj enj Awaiting requested review from enj

@soltysh soltysh Awaiting requested review from soltysh

1 more reviewer

@abhinavdahiya abhinavdahiya abhinavdahiya left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@wking @openshift-ci-robot @smarterclayton @openshift-bot @abhinavdahiya