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 a few typos #250

Merged
merged 2 commits into from Jun 14, 2020
Merged

Fix a few typos #250

merged 2 commits into from Jun 14, 2020

Conversation

kinow
Copy link
Contributor

@kinow kinow commented Jun 13, 2020

Closes #249

@rmk135 found a few more typos beside the one from #249.

Cheers
Bruno

@coveralls
Copy link

@coveralls coveralls commented Jun 13, 2020

Coverage Status

Coverage remained the same at 93.501% when pulling f12ec74 on kinow:fix-typos into fac7684 on ets-labs:develop.

@kinow
Copy link
Contributor Author

@kinow kinow commented Jun 13, 2020

Added a second commit with more changes, now to the documentation. Renamed one example class too. Happy to undo changes, or include new ones to the PR if necessary. Thanks for python-dependency-injector!

Copy link
Member

@rmk135 rmk135 left a comment

Hi @kinow ,

I've put a couple of comments. Please, take a look. Also, please, point the PR to develop branch.

Best,
Roman

@@ -160,7 +160,7 @@ aggregates other :py:class:`Factory` providers.

:py:class:`FactoryAggregate` is not overridable. Calling of
:py:meth:`FactoryAggregate.override` will result in raising of an
expection.
expectation.
Copy link
Member

@rmk135 rmk135 Jun 14, 2020

Choose a reason for hiding this comment

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

Change to "exception", please.

Copy link
Contributor Author

@kinow kinow Jun 14, 2020

Choose a reason for hiding this comment

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

Done!

@@ -9,7 +9,7 @@ This gives opportunity to make system behaviour more flexible at some point.
The main feature is that while your code is using providers, it depends on
providers, but not on the objects that providers provide. As a result of this,
you can change providing by provider object to a different one, but still
compatible one, without chaning your previously written code.
compatible one, without chaining your previously written code.
Copy link
Member

@rmk135 rmk135 Jun 14, 2020

Choose a reason for hiding this comment

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

Change to "changing", please.

Copy link
Contributor Author

@kinow kinow Jun 14, 2020

Choose a reason for hiding this comment

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

Done!

@rmk135
Copy link
Member

@rmk135 rmk135 commented Jun 14, 2020

That's cool. Thank you for the help. Please, take a look at the review comments.

@kinow kinow changed the base branch from master to develop Jun 14, 2020
rmk135
rmk135 approved these changes Jun 14, 2020
Copy link
Member

@rmk135 rmk135 left a comment

Looks good!

@rmk135 rmk135 merged commit 910dec3 into ets-labs:develop Jun 14, 2020
@kinow
Copy link
Contributor Author

@kinow kinow commented Jun 14, 2020

Edited the commit to fix the docs, and now pointing to develop branch. Thanks for the review @rmk135 !

@kinow kinow deleted the fix-typos branch Jun 14, 2020
@rmk135
Copy link
Member

@rmk135 rmk135 commented Jun 14, 2020

@kinow can you leave any comment at #249 . I want to assign it to you before closing. That's github "feature" - no way to assign an issue to somebody who is not involved at least with a comment :)

@kinow
Copy link
Contributor Author

@kinow kinow commented Jun 14, 2020

Done @rmk135 , and thanks for telling me about this feature. I thought only members of a team in an org/repo could be assignees. Good to know!

@rmk135
Copy link
Member

@rmk135 rmk135 commented Jun 14, 2020

Yep, I also didn't know this before. Googled and found this one - https://stackoverflow.com/questions/41171882/how-to-assign-an-issue-to-non-contributors

I've added you to the list list of contributors that is distributed with an every copy - https://github.com/ets-labs/python-dependency-injector/blob/develop/CONTRIBUTORS.rst

Also I've made a note in changelog - https://github.com/ets-labs/python-dependency-injector/blob/develop/docs/main/changelog.rst#development-version

Changes will appear on PyPi and ReadTheDocs in a couple of days.

Sincere Thanks,
Roman

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.

3 participants