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
[WFLY-17183] Fix the description of the filesystem-realm update-key-p… #16264
Conversation
Hi @bstansberry @fjuma could you please review this PR ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @anilabhabaral! Just have some small comments.
[source, options="nowrap"] | ||
---- | ||
/subsystem=elytron/filesystem-realm=fsRealm:update-key-pair(key-store=newKeyStore, key-store-alias=newKeyStoreAlias) | ||
/subsystem=elytron/filesystem-realm=keystore:write-attribute(name=key-store, value=newKeystore) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be consistent with the name of the filesystem-realm
used in the other examples, we should use filesystem-realm=fsRealm
here instead of filesystem-realm=keystore
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
[source, options="nowrap"] | ||
---- | ||
/subsystem=elytron/filesystem-realm=fsRealm:update-key-pair(key-store=newKeyStore, key-store-alias=newKeyStoreAlias) | ||
/subsystem=elytron/filesystem-realm=keystore:write-attribute(name=key-store, value=newKeystore) | ||
/subsystem=elytron/filesystem-realm=keystore:write-attribute(name=key-store-alias, value=newKeystoreAlias) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, filesystem-realm=fsRealm
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @anilabhabaral, looks good! Please squash the commits and then this should be good to go.
Hi @fjuma I am facing some issue while squashing the commits in my local repo. I think repository's manager can squash all the commits in a pull request into a single commit by selecting "Squash and merge" on a pull request.It will be helpful if you could do this for me. |
Hey @anilabhabaral, it looks the main branch was merged into yours, instead of the other way around. You'll need to delete the merge commit, and use an interactive rebase to squash the existing commits into a single one:
A quick summary of commands: git checkout WFLY-17183
git reset HEAD~1 --hard
git checkout main
git pull
git checkout WFLY-17183
git rebase -i main
# replace `pick` with `fixup` in all but the first commit, save and exit the file
git push -f Let me know if you have any questions. |
785d39b
to
c49796b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much @cam-rod for helping and thanks @anilabhabaral for the update! Looks good now.
Looks good @anilabhabaral, thanks! |
Hello @bstansberry could you please check whether this PR is ready for merge ? |
afa5be1
to
6d36fda
Compare
@anilabhabaral Can you use 'git commit --amend' and fix the commit message, i.e. just the WFLY-17183 line? The current first line is what people reading history see and it's not useful info. Thanks. Please ping @fjuma when ready; she is a WildFly merger and can merge it. |
…air operation in the Elytron documentation
6d36fda
to
77c3053
Compare
Thank you @bstansberry . I modified the commit message. Hello @fjuma could you please check whether this PR is ready for merge ? |
Thanks @fjuma |
Jira: https://issues.redhat.com/browse/WFLY-17183