Skip to content

Add proposal of robot account#35

Merged
steven-zou merged 2 commits into
goharbor:masterfrom
wy65701436:proposal-robot
Feb 12, 2019
Merged

Add proposal of robot account#35
steven-zou merged 2 commits into
goharbor:masterfrom
wy65701436:proposal-robot

Conversation

@wy65701436
Copy link
Copy Markdown
Contributor

@wy65701436 wy65701436 commented Jan 4, 2019

Signed-off-by: wang yan wangyan@vmware.com

nlowe
nlowe reviewed Jan 7, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@nlowe nlowe left a comment

Choose a reason for hiding this comment

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

There's quite a few open issues for this IIRC, and I know I'd definitely use this feature. Just one question on how logins work and a Q on the UI design.

Comment thread proposals/new/robot.md Outdated
Comment thread proposals/new/robot.md
@steven-zou steven-zou added kind/proposal area/CI&CD Related with CI/CD area/auth Any thing related with auth labels Jan 9, 2019
nlowe
nlowe reviewed Jan 9, 2019
View reviewed changes
Comment thread proposals/new/robot.md
nlowe
nlowe reviewed Jan 9, 2019
View reviewed changes
Comment thread proposals/new/robot.md Outdated
@wy65701436 wy65701436 mentioned this pull request Jan 15, 2019
Closed
reasonerjt
reasonerjt reviewed Jan 15, 2019
View reviewed changes
Comment thread proposals/new/robot.md Outdated
reasonerjt
reasonerjt reviewed Jan 15, 2019
View reviewed changes
Comment thread proposals/new/robot.md Outdated
reasonerjt
reasonerjt reviewed Jan 15, 2019
View reviewed changes
Comment thread proposals/new/robot.md
reasonerjt
reasonerjt reviewed Jan 15, 2019
View reviewed changes
Comment thread proposals/new/robot.md
reasonerjt
reasonerjt reviewed Jan 15, 2019
View reviewed changes
Comment thread proposals/new/robot.md
reasonerjt
reasonerjt reviewed Jan 15, 2019
View reviewed changes
Comment thread proposals/new/robot.md Outdated
@steven-zou
Copy link
Copy Markdown
Contributor

steven-zou commented Jan 16, 2019

@nlowe @cd1989

If no further comments, let's +1 to merge it into the master.

@steven-zou
Copy link
Copy Markdown
Contributor

steven-zou commented Jan 17, 2019

@wy65701436

DCO missing.

nlowe
nlowe previously approved these changes Jan 17, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@nlowe nlowe left a comment

Choose a reason for hiding this comment

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

While I still think we should seriously consider using a server-wide configured username instead of prefixes for docker login, I don't think that should block this PR and can be addressed later. Overall, LGTM.

@steven-zou
Copy link
Copy Markdown
Contributor

steven-zou commented Jan 17, 2019

While I still think we should seriously consider using a server-wide configured username instead of prefixes for docker login, I don't think that should block this PR and can be addressed later. Overall, LGTM.

@nlowe

I do not see the significant meaningful benefits of replacing the fix robot user name robot$ with the configurable username. Let's see if we can get more same requirements from the community after user trying it.

steven-zou
steven-zou previously approved these changes Jan 17, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@steven-zou steven-zou left a comment

Choose a reason for hiding this comment

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

LGTM

@wy65701436 wy65701436 dismissed stale reviews from steven-zou and nlowe via e03fe98 January 18, 2019 00:46
nlowe
nlowe previously approved these changes Jan 18, 2019
View reviewed changes
zhoumeina
zhoumeina reviewed Jan 18, 2019
View reviewed changes
Comment thread proposals/new/robot.md Outdated
reasonerjt
reasonerjt reviewed Jan 18, 2019
View reviewed changes
Comment thread proposals/new/robot.md Outdated
@steven-zou
Copy link
Copy Markdown
Contributor

steven-zou commented Jan 18, 2019

@nlowe @cd1989

Can we merge this PR now?

@ericvmw ericvmw mentioned this pull request Jan 30, 2019
Closed
ghost
ghost suggested changes Feb 1, 2019
View reviewed changes
Comment thread proposals/new/robot.md
@ghost
Copy link
Copy Markdown

ghost commented Feb 1, 2019

Our DCO robot friend is unhappy. Do you mind both squashing commits and signing before we merge?

(note that there are a few questions / outstanding comments to address anyway)

wang yan added 2 commits February 11, 2019 14:08
@steven-zou steven-zou merged commit 871af4f into goharbor:master Feb 12, 2019
@steven-zou steven-zou added the status/accepted Proposal is accepted label Feb 16, 2019
@nlowe nlowe mentioned this pull request Mar 12, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reasonerjt reasonerjt reasonerjt left review comments

@steven-zou steven-zou steven-zou left review comments

+3 more reviewers

@zhoumeina zhoumeina zhoumeina left review comments

@nlowe nlowe nlowe left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@reasonerjt reasonerjt

@steven-zou steven-zou

@ywk253100 ywk253100

Labels

area/auth Any thing related with auth area/CI&CD Related with CI/CD kind/proposal status/accepted Proposal is accepted

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@wy65701436 @steven-zou @nlowe @reasonerjt @zhoumeina @ywk253100