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

Add test case to document env header contains whitespace error as expected behavior. #5156

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MicahKimel
Copy link

- What I did

- How to verify it

  • Running test cases docker buildx bake test looked good to me

- Description for the changelog

Test case documenting using a bad header in .env will throw a error is expected behavior.

Accidentally closed [#5081], so re-opened here. Please let me know any feedback @Benehiko!

@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.73%. Comparing base (64206ae) to head (dd80599).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5156      +/-   ##
==========================================
- Coverage   61.76%   61.73%   -0.04%     
==========================================
  Files         297      294       -3     
  Lines       20768    20763       -5     
==========================================
- Hits        12828    12817      -11     
- Misses       7024     7027       +3     
- Partials      916      919       +3     

Copy link
Member

@Benehiko Benehiko left a comment

Choose a reason for hiding this comment

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

LGTM

Could you squash all of the commits you made on this branch into a singular commit?

⋊> ~/G/docker-cli on b9ba41a8b  git log --oneline                                                                                           
b9ba41a8b (HEAD, MichaKimel/master) remove assert
1d5b2325c fix container_run.md spacing
b2b2bacf7 Add test parse env formatted badly with spaces
d69d501f6 Merge pull request #5155 from vvoland/docker-27.0-rc
...

You would want to rebase until d69d501 and squash b9ba41a, 1d5b232.

git rebase -i d69d501f6
pick b2b2bacf7 Add test parse env formatted badly with spaces
-pick 1d5b2325c fix container_run.md spacing
+squash 1d5b2325c fix container_run.md spacing
-pick b9ba41a8b remove assert
+squash b9ba41a8b remove assert

Save then edit the commit message, remove the redundant parts and make sure you have the Signed-off-by: MicahKimel <micah.kimel@my.wheaton.edu>.

Then force push, git push -f

fix container_run.md spacing

Signed-off-by: MicahKimel <micah.kimel@my.wheaton.edu>
@MicahKimel
Copy link
Author

Thank you very much @Benehiko, now I understand how to rebase!! I went ahead and merged with upstream.

@Benehiko
Copy link
Member

Hey @MicahKimel, thank you for taking the time to work on this. I see you have a merge commit from master into your fork's master. This should unfortunately be corrected since the project prefers rebasing your commits on top of master - this ensures a linear history on master after the PR is merged.

https://github.com/docker/cli/blob/master/CONTRIBUTING.md#conventions
https://git-scm.com/docs/git-rebase

You will most likely need to revert the last git merge upstream/master that you did and then instead run git rebase upstream/master.

Something like this (please also check this on your own):

// reset the last commit (the git merge upstream/master) (destructive!)
git reset --hard HEAD~1
// ensure you have everything from upstream
git fetch --all
// do a rebase on top of upstream/master
git rebase upstream/master
// force push to your fork's master (destructive!)
git push -f
``

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.

docker run throws error if an env var contains whitespace
3 participants