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

[SHIRO-805] Spelling #269

Merged
merged 1 commit into from
Aug 4, 2021
Merged

[SHIRO-805] Spelling #269

merged 1 commit into from
Aug 4, 2021

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Dec 22, 2020

This PR corrects misspellings identified by the check-spelling action.

The misspellings have been reported at jsoref@c0e3b03#commitcomment-45334058

The action reports that the changes in this PR would make it happy: jsoref@276884c

Note: this PR does not include the action. If you're interested in running a spell check on every PR and push, that can be offered separately.

Following this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SHIRO-XXX] - Fixes bug in SessionManager,
    where you replace SHIRO-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install apache-rat:check to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If you have a group of commits related to the same change, please squash your commits into one and force push your branch using git rebase -i.

Trivial changes like typos do not require a JIRA issue (javadoc, comments...).
In this case, just format the pull request title like (DOC) - Add javadoc in SessionManager.

If this is your first contribution, you have to read the Contribution Guidelines

If your pull request is about ~20 lines of code you don't need to sign an Individual Contributor License Agreement
if you are unsure please ask on the developers list.

To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

I expect to squash this PR near the end. But it's much easier for me to address review comments before squashing rather than after, as they generally address individual word families.

If there are files that you don't want changed, please let me know -- dropping them is fairly easy.

@bmarwell
Copy link
Contributor

Thanks!
Can you kindly open a jira issue for this?

Btw, some classes get renamed. If they are not in a test folder, we'd need to let them misspelled the way they are.

@jsoref jsoref changed the title Spelling [SHIRO-805] Spelling Dec 22, 2020
@jsoref
Copy link
Contributor Author

jsoref commented Dec 22, 2020

For the curious, this forced push just folded 8fce45c into 8fd6e14.

It sounds like I'm going to split this PR into a couple of pieces, and it's helpful if my commits are reasonably labeled before I do that.

@jsoref
Copy link
Contributor Author

jsoref commented Dec 22, 2020

This is now roughly:

  • tests
  • comments
  • docs

The commits I've dropped are variable/function/class renames. Note for simplicity, if a code rename includes a comment change, that was included.

@fpapon
Copy link
Member

fpapon commented Feb 19, 2021

@jsoref can you squash the commits please?

@jsoref
Copy link
Contributor Author

jsoref commented Feb 19, 2021

(This is just a rebased version before squashing.)

@jsoref jsoref force-pushed the spelling branch 2 times, most recently from e2f648b to dcb9e4b Compare February 19, 2021 15:40
@jsoref
Copy link
Contributor Author

jsoref commented Apr 12, 2021

@fpapon ?

@bmarwell
Copy link
Contributor

Maybe just #269 (comment) left, then we are good imho.

@fpapon fpapon self-requested a review April 14, 2021 16:08
Copy link
Member

@fpapon fpapon left a comment

Choose a reason for hiding this comment

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

LGTM

@fpapon fpapon requested review from bdemers and bmarwell April 16, 2021 09:45
Copy link
Contributor

@bmarwell bmarwell left a comment

Choose a reason for hiding this comment

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

some nits

* authentication state that might exist, thereby effectively "unauthenticating" the {@code Subject}.
* authentication state that might exist, thereby effectively removing the "authenticated" state of the {@code Subject}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not 100% sure about this one. I know "to unauthenticate" is not a common word, but it is easier to understand than "removing the >authenticated<-state".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unauthenticate really isn't a word. Conceptually the word would be deauthenticate (but even that isn't in Google Chrome's dictionary -- nor is it in the dictionary I'm using, although I'd be ok w/ it).

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe @bdemers can think of something else?

@jsoref
Copy link
Contributor Author

jsoref commented Aug 2, 2021

Fwiw, I'm trying to update this PR, I have everything resolved, but there's quite a bit of additional drift between when I started and now... So, I'm also trying to get my tool to give new happier output.

@bmarwell
Copy link
Contributor

bmarwell commented Aug 2, 2021

Fwiw, I'm trying to update this PR, I have everything resolved, but there's quite a bit of additional drift between when I started and now... So, I'm also trying to get my tool to give new happier output.

Sorry it took us so long. Shouldn't happen again! :)

* account
* against
* and
* application
* authentication
* authorization
* authorizer
* automatically
* axes
* both
* build
* case
* consistent
* contains
* control
* controls
* created
* deauthenticating
* defaults
* defined
* delimited
* delimiter
* deserialized
* deserializes
* e.g.
* efficient
* encapsulate
* encryption
* environment
* et al.
* etc.
* evaluate
* evaluator
* exception
* explicitly
* false
* filters
* from
* hash format
* hashed
* hierarchy
* i.e.
* identifier
* identifying
* immediately
* implementation
* implementers
* inferring
* injectable
* iterator
* join point
* manner
* modifier
* nonexistent
* obtained
* overridden
* password
* performing
* permission
* permissions
* preemptively
* recognized
* reget
* remnant
* request
* required
* resolvable
* returned
* safely
* session
* subsequent
* successfully
* type
* undertow
* usually
* wield

Signed-off-by: Josh Soref <[email protected]>
@@ -159,8 +159,8 @@ protected AuthorizationInfo buildAuthorizationInfo(Set<String> roleNames) {
Set<String> roleNames;
roleNames = new LinkedHashSet<String>();

SearchControls searchCtls = new SearchControls();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not strictly necessary, but ok for me.

@fpapon fpapon merged commit 0c0d9da into apache:main Aug 4, 2021
@fpapon
Copy link
Member

fpapon commented Aug 4, 2021

@jsoref thanks for your contribution!

@jsoref jsoref deleted the spelling branch August 4, 2021 15:53
@sunshineandy
Copy link

sunshineandy commented Aug 5, 2021 via email

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.

5 participants