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.

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

don't use null values in the bake definition #4122

Merged
merged 1 commit into from
Mar 27, 2023
Merged

don't use null values in the bake definition #4122

merged 1 commit into from
Mar 27, 2023

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Mar 27, 2023 •
edited by thaJeztah
Loading

- What I did

Remove null values in the bake definition to avoid issues with environments with old Buildx version (< 0.10).

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added this to the v-next milestone Mar 27, 2023
thaJeztah
thaJeztah approved these changes Mar 27, 2023
View reviewed changes
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.

LGTM

@crazy-max
Copy link
Member Author

e2e tests are not in good shape: https://github.com/docker/cli/actions/runs/4531228785/jobs/7981128986?pr=4122#step:5:850

=== FAIL: e2e/trust TestSignLocalImage (0.15s)
    sign_test.go:32: assertion failed: 
        Command:  docker trust sign registry:5000/sign:v1
        ExitCode: 1
        Error:    exit status 1
        Stdout:   
        Stderr:   Error: error contacting notary server: x509: certificate has expired or is not yet valid: current time 2023-03-27T10:13:42Z is after 2023-03-25T14:47:57Z

@thaJeztah
Copy link
Member

OH! Yes, that was the other one I found in #4121 (comment). I started to look at that, when the whole Windows Defender whack-a-mole began 😂

@crazy-max crazy-max mentioned this pull request Mar 27, 2023
Merged
@thaJeztah
Copy link
Member

can you try rebasing?

@crazy-max
Copy link
Member Author

can you try rebasing?

Can't atm: https://www.githubstatus.com/incidents/52z0j6phhnjs

@thaJeztah
Copy link
Member

AH! That explained why I couldn't push. I tried, but it failed, then saw you were out of your meeting

1 similar comment
@thaJeztah
Copy link
Member

AH! That explained why I couldn't push. I tried, but it failed, then saw you were out of your meeting

@thaJeztah thaJeztah mentioned this pull request Mar 27, 2023
Merged
@crazy-max
Copy link
Member Author

@thaJeztah Should be good now

@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2023 •
edited
Loading

Codecov Report

Merging #4122 (bec5d37) into master (7f11449) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #4122   +/-   ##
=======================================
  Coverage   59.16%   59.16%           
=======================================
  Files         287      287           
  Lines       24716    24716           
=======================================
  Hits        14623    14623           
  Misses       9209     9209           
  Partials      884      884

@thaJeztah
Copy link
Member

CI is happy 🎉

@thaJeztah thaJeztah merged commit 2165386 into docker:master Mar 27, 2023
@crazy-max crazy-max deleted the fix-bake branch March 27, 2023 14:24
@thaJeztah thaJeztah mentioned this pull request Mar 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees
No one assigned
Labels
area/packaging process/cherry-picked status/2-code-review
Projects
None yet
Milestone
24.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@crazy-max @thaJeztah @codecov-commenter