Skip to content
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.

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

fix: TestWorkflowStepRetry's comment accurately reflects what it does. #9234

Merged
merged 3 commits into from
Aug 11, 2022

Conversation

robertkotcher
Copy link
Contributor

Update TestWorkflowStepRetry's comment to accurately reflect what it does.

Signed-off-by: rkotcher robert@synthesis.ai

Signed-off-by: Robert Kotcher <robert@synthesis.ai>
@robertkotcher
Copy link
Contributor Author

Looks like the Docs failure is unrelated to changes made in this PR 🤔

@robertkotcher
Copy link
Contributor Author

@juliev0 it looks like you have approved, but github doesn't seem to recognize that since it still says:

Merging can be performed automatically with 1 approving review

Have you seen this before? Also assuming you have write access? (looks like that is also a requirement)

Thank you!

@juliev0
Copy link
Contributor

juliev0 commented Jul 28, 2022

@juliev0 it looks like you have approved, but github doesn't seem to recognize that since it still says:

Merging can be performed automatically with 1 approving review

Have you seen this before? Also assuming you have write access? (looks like that is also a requirement)

Thank you!

You guessed it. I don't have permission. That "1 approving review" message is confusing. I'm a member of argo-workflows but not yet an "approver".

@robertkotcher
Copy link
Contributor Author

@sarabala1979 you assigned to @juliev0 but it seems somebody with write perms will also need to approve. Thanks!

@robertkotcher
Copy link
Contributor Author

@sarabala1979 seems as if somebody else will also have to click the merge button. when does that happen? (thx, just trying to learn the process here :) )

@juliev0
Copy link
Contributor

juliev0 commented Aug 11, 2022

@sarabala1979 seems as if somebody else will also have to click the merge button. when does that happen? (thx, just trying to learn the process here :) )

Hey Robert. Sorry for the delay. Unfortunately, some of these PRs aren't getting immediate attention. I'll bring it up to him tomorrow and hopefully we can just merge it.

@robertkotcher
Copy link
Contributor Author

@sarabala1979 seems as if somebody else will also have to click the merge button. when does that happen? (thx, just trying to learn the process here :) )

Hey Robert. Sorry for the delay. Unfortunately, some of these PRs aren't getting immediate attention. I'll bring it up to him tomorrow and hopefully we can just merge it.

No worries :) just trying to learn the process as I'm hoping to contribute with more meaningful PRs in the future. Thanks @juliev0

@sarabala1979 sarabala1979 merged commit 61f252f into argoproj:master Aug 11, 2022
@sarabala1979
Copy link
Member

@robertkotcher sorry for the delay

@juliev0
Copy link
Contributor

juliev0 commented Aug 11, 2022

Thanks, @sarabala1979!

@sarabala1979 seems as if somebody else will also have to click the merge button. when does that happen? (thx, just trying to learn the process here :) )

Hey Robert. Sorry for the delay. Unfortunately, some of these PRs aren't getting immediate attention. I'll bring it up to him tomorrow and hopefully we can just merge it.

No worries :) just trying to learn the process as I'm hoping to contribute with more meaningful PRs in the future. Thanks @juliev0

Great @robertkotcher!

Thanks @sarabala1979 !

juchaosong pushed a commit to juchaosong/argo-workflows that referenced this pull request Nov 3, 2022
argoproj#9234)

Signed-off-by: Robert Kotcher <robert@synthesis.ai>

Signed-off-by: Robert Kotcher <robert@synthesis.ai>
Co-authored-by: Robert Kotcher <robert@synthesis.ai>
Signed-off-by: juchao <juchao@coscene.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants