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

Docs minor fixes #1183

Merged
merged 2 commits into from Aug 19, 2021
Merged

Docs minor fixes #1183

merged 2 commits into from Aug 19, 2021

Conversation

pirj
Copy link
Member

@pirj pirj commented Aug 14, 2021

Broken note

A typographic quote in

`Include`’s

seems to have broken the parsing of backticks. You may see that the text between Includes is wrapped into monospace code section, and the second backtick of the second backtick on the last Includes is missing.

image

We have had this issue with regular single quotes, that's why we've used a typographic one here. But the problems either affects both, or it started to with Antora version update.

Broken sub-department cops docs

If you go to https://docs.rubocop.org/rubocop-rspec/2.0/upgrade_to_version_2.html, links to the second-level cops docs are broken. The last one is correct, https://docs.rubocop.org/rubocop-rspec/2.0/cops_rspec.html, while others have anchors, e.g. https://docs.rubocop.org/rubocop-rspec/2.0/upgrade_to_version_2.html#cops_capybara.adoc. This is probably due to cops_docs.adoc vs cops_docs/ name clash.
Suspect: https://docs.antora.org/antora/2.3/page/page-id/#important
Ideally, we would generate cops_rspec_capybara.adoc instead of cops_rspec/capybara.adoc.

I've tracked it down to this method in RuboCop:

      def department_to_basename(department)
          "cops_#{department.downcase}"

Any suggestions? I've sent a PR to RuboCop to update this to change downcase to downcase.tr('/', '_').
Other RuboCop extensions don't seem to make use of sub-departments, and will be unaffected by this change.
Docs in the second commit are generated using the patched RuboCop.


Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • [-] Added tests.
  • Updated documentation.
  • [-] Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
  • [-] The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

@pirj pirj self-assigned this Aug 14, 2021
@pirj
Copy link
Member Author

pirj commented Aug 15, 2021

Corresponding RuboCop PR is merged. After its release:

  • the build will become green
  • in development, we won't have to point to github: inGemfile.local
  • ...
  • and the doc will be fixed!

@bquorning
Copy link
Collaborator

This PR should not be merged until the next version of RuboCop has been released, right?

@pirj
Copy link
Member Author

pirj commented Aug 15, 2021

This PR should not be merged until the next version of RuboCop has been released, right?

We can merge it, but that would cause complication for contributors.
In any case, contributors with the checked-out rubocop-rspec would have to bundle update --conservative rubocop to use the fixed task. Or we would have to bump our dependency again. I don't have a preference here, maybe it would make sense to bump the dependency and make everyone's live simpler.

@bquorning
Copy link
Collaborator

A new version of RuboCop was released today – I have rerun our CI, and everything is green.

If a contributor runs into tests failing because of an old version of RuboCop, I think they will quickly find the cause. If not, we’re here to help (if they ask).

I don’t think this is enough cause to bump the gem’s requirement of RuboCop.

@pirj pirj merged commit 2ff7294 into master Aug 19, 2021
@pirj pirj deleted the minor-doc-fixes branch August 19, 2021 11:59
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