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

🎨 fix typos #2898

Merged
merged 4 commits into from
Apr 18, 2024
Merged

🎨 fix typos #2898

merged 4 commits into from
Apr 18, 2024

Conversation

ff137
Copy link
Contributor

@ff137 ff137 commented Apr 17, 2024

The most minor of quality of life improvements.

The motivation was to fix typos that appear in the openapi spec, because typos in the model descriptions would propagate to autogenerated clients.

Fixing those, I would 'find all' to fix all instances in the codebase. So at first it was just to fix typos in model descriptions, and that expanded to more parts.
When I started correcting double spaces, that's when I knew I'd gone too far.

Using the Code Spell Checker vscode extension made this pretty straightforward. Can recommend.

No external model names or anything changed, so no breaking changes as a result of corrections. Just internal variable names, comments, and some readme improvements.

Edit: I also modified model descriptions to no longer contain \n characters or excessive whitespace, and committed the updated openapi specs, to reflect the changes there as well.

Edit2: Went a bit above and beyond to correct typos in more modules. Lots of files changed, I know... but a scanning review should just take a couple minutes!

@ff137 ff137 force-pushed the fix/typos branch 2 times, most recently from a7c1fb7 to 53cdedd Compare April 17, 2024 13:15
@ff137 ff137 marked this pull request as draft April 17, 2024 15:12
@ff137 ff137 marked this pull request as ready for review April 17, 2024 15:52
dbluhm
dbluhm previously approved these changes Apr 17, 2024
@dbluhm
Copy link
Member

dbluhm commented Apr 17, 2024

Looks like a regen of the openapi and swagger json files is necessary after merging #2899 🙂

Signed-off-by: ff137 <ff137@proton.me>
Signed-off-by: ff137 <ff137@proton.me>
Signed-off-by: ff137 <ff137@proton.me>
Copy link

sonarcloud bot commented Apr 18, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@@ -64,7 +64,7 @@ async def _resolve(
return doc

async def create_and_store(self, profile: Profile, peer2: str):
"""Injest did:peer:2 create did:peer:3 and store document."""
"""Inject did:peer:2 create did:peer:3 and store document."""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Inject did:peer:2 create did:peer:3 and store document."""
"""Ingest did:peer:2 create did:peer:3 and store document."""

This one might actually be meant to be "ingest," from the context; i.e. taking in a did:peer:2, creating a did:peer:3, and storing a document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ingest was my first guess 😄 but then I doubted myself when I noticed that "inject" is also just 1 character away. Plus, inject is less gastrointestinal.

If ingest makes more sense, I'll amend that 👌

The other funny one was "encluding", which was the hardest to correct 😂 both "including" and "excluding" are just a 1 character diff. So that's another one I'd appreciate confirmation that I guessed right 😄

Copy link
Member

Choose a reason for hiding this comment

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

I think it's probably clear enough as it is 🙂 The wording in general on this one is a bit odd

@dbluhm dbluhm merged commit 9243ef3 into hyperledger:main Apr 18, 2024
8 checks passed
@ff137 ff137 deleted the fix/typos branch April 24, 2024 14:20
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.

None yet

2 participants