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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[READY FOR REVIEW] MONGOID-5570 Code Docs: 100% class/method documentation coverage 馃挴馃帀 #5546

Closed

Conversation

johnnyshields
Copy link
Contributor

@johnnyshields johnnyshields commented Feb 16, 2023

This PR adds 100% coverage of class and method documentation.

This PR contains only doc comments; code was modified.

It also enables the following Rubocop cops, which will ensure coverage stays at 100%:

@johnnyshields johnnyshields changed the title Rubocop: Add missing class documentation (WIP) Rubocop: Add missing documentation (WIP) Feb 16, 2023
@johnnyshields johnnyshields marked this pull request as draft February 16, 2023 21:02
@johnnyshields johnnyshields changed the title Rubocop: Add missing documentation (WIP) Rubocop: Add missing class/method documentation (WIP) Feb 16, 2023
@@ -20,7 +20,7 @@ def standard_dependencies

group :development, :test do
gem 'rspec-core', '~> 3.10'
gem 'rubocop', '~> 1.45.1'
gem 'rubocop', git: 'https://github.com/rubocop/rubocop.git'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can be '~> 1.45.2' when released. Related to this fix: rubocop/rubocop@b46b9b5 which was discovered in the process of making this PR.

@johnnyshields johnnyshields marked this pull request as ready for review February 18, 2023 17:31
@johnnyshields johnnyshields changed the title Rubocop: Add missing class/method documentation (WIP) [READY FOR REVIEW] Add 100% class/method documentation coverage Feb 18, 2023
@johnnyshields johnnyshields changed the title [READY FOR REVIEW] Add 100% class/method documentation coverage [READY FOR REVIEW] 100% class/method documentation coverage 馃挴馃帀 Feb 18, 2023
@johnnyshields johnnyshields changed the title [READY FOR REVIEW] 100% class/method documentation coverage 馃挴馃帀 [READY FOR REVIEW] MONGOID-5570 - 100% class/method documentation coverage 馃挴馃帀 Feb 18, 2023
@johnnyshields johnnyshields changed the title [READY FOR REVIEW] MONGOID-5570 - 100% class/method documentation coverage 馃挴馃帀 [READY FOR REVIEW] MONGOID-5570 Code Docs: 100% class/method documentation coverage 馃挴馃帀 Feb 19, 2023
@alexbevi
Copy link
Contributor

alexbevi commented Feb 28, 2023

@johnnyshields this is a change we would prefer to introduce incrementally. MONGOID-5304 was merged and introduces static analysis, however we'd like to take some time to evaluate our options before enabling additional cops.

I'm going to close this PR and MONGOID-5570 for the time being and we'll add additional tickets to the backlog as we introduce additional cops.

We appreciate the amount of effort invested in updating all these source files, however we cannot accept this PR presently.

@alexbevi alexbevi closed this Feb 28, 2023
@johnnyshields
Copy link
Contributor Author

@alexbevi would you accept this PR if I remove the Rubocop enablement? Documenting all these methods took a considerable amount of time and research and it would be a shame for it to go to waste.

@johnnyshields
Copy link
Contributor Author

@alexbevi I've raised #5559 with the Rubocop changes removed.

@alexbevi
Copy link
Contributor

@alexbevi I've raised #5559 with the Rubocop changes removed.

I'll discuss with the team, however we would need to validate each entry for accuracy as well so the time to review and merge such a large PR would not be insignificant. We may consider "peeling off" chunks of the PR incrementally as those code files are updated instead.

@johnnyshields
Copy link
Contributor Author

Peeling off chunks is fine by me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants