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 tests to support both log plugin feedbacks #2549

Merged
merged 1 commit into from Apr 22, 2020

Conversation

wpjunior
Copy link
Contributor

I am improving the feedback when docker uses a log driver, could you see the change at: moby/moby#40807.

After above change I broke one test that is inside this project, then, I am sending the fix.

Thanks.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

I'm not a python code, but this seems sane to me, thanks!

LGTM

@thaJeztah
Copy link
Member

Looks like tests are failing on something else (unrelated); @ulyssessouza could you have a look?

@ulyssessouza ulyssessouza self-assigned this Apr 20, 2020
@ulyssessouza
Copy link
Collaborator

Looks like it was just a flaky test... The only thing missing is to have the dco-signed

@wpjunior wpjunior force-pushed the fix-plugin-feedback branch 2 times, most recently from 91c93c0 to 390aca3 Compare April 21, 2020 19:45
Signed-off-by: Wilson Júnior <wilsonpjunior@gmail.com>
Docker-DCO-1.1-Signed-off-by: Wilson Júnior <wilsonpjunior@gmail.com> (github: wpjunior)
@wpjunior
Copy link
Contributor Author

Hi @ulyssessouza how can I complete dco-signed check ?, I signed my commits with related signature and it is not work.

@thaJeztah
Copy link
Member

I think only the first one is supported (the Docker-DCO-1.1-Signed-off-by: was something from the past, and the "DCO" bot won't understand it.

Can you remove the last (Docker-DCO...) line? I think after that it should be correct

@thaJeztah
Copy link
Member

Oh! Besides that, @ulyssessouza I see you enabled the required option for the DCO check; unfortunately that doesn't work; there's a bug in GitHub's handling of those checks; if you add it to a repository that has open PR's and you set it to "required", it never completes (gets stuck in "pending").

If you disable the "required" check, things should pass

@thaJeztah
Copy link
Member

Unfortunately, I don't have write/admin access on this repository otherwise I'd have updated that setting, but @wpjunior if you can update the commit message, then things should be OK from your side 🤗

@ulyssessouza ulyssessouza merged commit 9a24df5 into docker:master Apr 22, 2020
@ulyssessouza
Copy link
Collaborator

Got it merged!

@wpjunior Obrigado pela PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants