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

[CI:DOCS] Documented that uidmap and guidmap are based on subgid subuid mapping #8695

Merged
merged 1 commit into from Dec 23, 2020
Merged

[CI:DOCS] Documented that uidmap and guidmap are based on subgid subuid mapping #8695

merged 1 commit into from Dec 23, 2020

Conversation

topas-rec
Copy link
Contributor

@topas-rec topas-rec force-pushed the document_uid_gid_map_is_relative_to_subuid_subgid_mapping_fixes_6123 branch from 1cd09b8 to 140945e Compare December 14, 2020 07:05
@topas-rec
Copy link
Contributor Author

@TomSweeneyRedHat your suggestions are applied now. Thanks for reviewing.

@zhangguanzhang
Copy link
Collaborator

zhangguanzhang commented Dec 14, 2020 •
edited

Thank you for your code contribution, you need sign your info in your git commit messages.

@topas-rec topas-rec force-pushed the document_uid_gid_map_is_relative_to_subuid_subgid_mapping_fixes_6123 branch from 140945e to 6bd0c6f Compare December 15, 2020 19:30
@topas-rec
Copy link
Contributor Author

Thank you for your code contribution, you nedd sign your info in your git commit messages.

@zhangguanzhang is this done now?

@TomSweeneyRedHat
Copy link
Member

@topas-rec I'm not sure you signed the commit yet. Did you do git commit --amend-s and then fetch, rebase and push?

@rhatdan
Copy link
Member

rhatdan commented Dec 16, 2020

@topas-rec Could you rebase this PR. The EXT Services tests needs a rebase to pass.

@topas-rec topas-rec force-pushed the document_uid_gid_map_is_relative_to_subuid_subgid_mapping_fixes_6123 branch from 6bd0c6f to 9ff8819 Compare December 16, 2020 19:52
@topas-rec
Copy link
Contributor Author

I use the git integration in eclipse. Signing always failed cause the secret key was not found to the public key.
I used the terminal instead and it did not fail. I was unable to enter the commit message in multiple lines.

Is it signed now?

@topas-rec
Copy link
Contributor Author

@topas-rec Could you rebase this PR. The EXT Services tests needs a rebase to pass.

Should be done

@rhatdan
Copy link
Member

rhatdan commented Dec 16, 2020

@topas-rec Still has two PRs Can you merge and make sure they are signed.

@topas-rec topas-rec force-pushed the document_uid_gid_map_is_relative_to_subuid_subgid_mapping_fixes_6123 branch 2 times, most recently from f8be73c to c1d5c35 Compare December 17, 2020 21:28
@topas-rec
Copy link
Contributor Author

I had a problem with the created gpg key. After recreating and using --amend instead of -a at commit command (and entering the correct passphrase) it worked.

git says signed now.

tobias@tobias-pc:~/DATA/Computers/Software_development/git/podman$ git log --show-signature -2
commit c1d5c350984e96bbd3020f3e0c7d5ad22f3a019a (HEAD -> document_uid_gid_map_is_relative_to_subuid_subgid_mapping_fixes_6123)
gpg: Signature made Do 17 Dez 2020 22:25:54 CET
gpg:                using RSA key A1FB7D9E543E834FE0996DD02D8179D609148CFA
gpg: Good signature from "Tobias Boesch <tobias.boesch@googlemail.com>" [ultimate]
Author: Tobias Boesch <tobias.boesch@googlemail.com>
Date:   Fri Dec 11 22:36:56 2020 +0100

    Documented that uidmap and guidmap are based on subgid subuid mapping * Closes #6123 Signed-off-by: Tobias Boesch <tobias.boesch@googlemail.com>

commit a17afa931d1aa73b8657cf26de3b49841837f66d (upstream/master)
gpg: Signature made Do 17 Dez 2020 18:22:37 CET
gpg:                using RSA key 4AEE18F83AFDEB23
gpg: Can't check signature: No public key
Merge: 033336606 86335aa4a
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Thu Dec 17 17:22:37 2020 +0000

    Merge pull request #8752 from baude/bindings3volumes
    
    misc bindings to podman v3
tobias@tobias-pc:~/DATA/Computers/Software_development/git/podman$

@topas-rec
Copy link
Contributor Author

Additionally I had to change the email from gmail to googlemail in github.

@rhatdan
Copy link
Member

rhatdan commented Dec 18, 2020

The validate can be a little picky, it wants a blank line between your signature and the tittle of your PR. The title needs to be less then 80 chars also.

The PR can also not have any whitespace.

@topas-rec
Copy link
Contributor Author

The validate can be a little picky, it wants a blank line between your signature and the tittle of your PR. The title needs to be less then 80 chars also.

The PR can also not have any whitespace.

Okay, is this something that prevents this PR from beeing merged?

@topas-rec topas-rec force-pushed the document_uid_gid_map_is_relative_to_subuid_subgid_mapping_fixes_6123 branch 2 times, most recently from 7d3404b to 2ca3bc2 Compare December 21, 2020 19:33
@rhatdan
Copy link
Member

rhatdan commented Dec 21, 2020

Yes all PRs have to pass the tests to get merged.

  • FAIL - has whitespace errors. See git show --check 2ca3bc213d3a2a9e16260abe2350551ad7ac85ce.
  • FAIL - does not have a valid DCO
  • FAIL - commit subject exceeds 90 characters

@topas-rec topas-rec force-pushed the document_uid_gid_map_is_relative_to_subuid_subgid_mapping_fixes_6123 branch 2 times, most recently from 74f8f46 to 096817f Compare December 22, 2020 21:16
@topas-rec
Copy link
Contributor Author

FAIL - has whitespace errors. See git show --check <commit id>

image

I wanted to add a new line like explained here.
This is not an issue I think. Now the tests are passed, but I intended to have a new line in the documentation - which is now gone

rhatdan
rhatdan approved these changes Dec 23, 2020
View reviewed changes
CONTRIBUTING.md Outdated
## Topics

* [Reporting Issues](#reporting-issues)
* [Reporting Issueqweqwes](#reporting-issues)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like typos have snuck in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

CONTRIBUTING.md Outdated
@@ -3,10 +3,10 @@

We'd love to have you join the community! Below summarizes the processes
that we follow.

asd
Copy link
Member

Choose a reason for hiding this comment

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

Typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typo?

Yes. Rermoved

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, topas-rec

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 23, 2020
@topas-rec
Document uid/gidmap are based on subuid/gid mapping
3cc0801 
* Closes containers#6123

Signed-off-by: Tobias Boesch <tobias.boesch@googlemail.com>
@topas-rec topas-rec force-pushed the document_uid_gid_map_is_relative_to_subuid_subgid_mapping_fixes_6123 branch from 096817f to 3cc0801 Compare December 23, 2020 10:48
@topas-rec
Copy link
Contributor Author

Thanks for your help so far.

@rhatdan
Copy link
Member

rhatdan commented Dec 23, 2020

/lgtm
Thanks @topas-rec

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 23, 2020
@openshift-merge-robot openshift-merge-robot merged commit 3728ca9 into containers:master Dec 23, 2020
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@rhatdan rhatdan rhatdan approved these changes

Assignees

@rhatdan rhatdan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Document that in rootless mode, uidmap is in terms of the default mapping, not host uids
6 participants
@topas-rec @zhangguanzhang @TomSweeneyRedHat @rhatdan @openshift-ci-robot @openshift-merge-robot