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

Improve "Reference" (and/or rationale in docs) for existing (and future) cops #97

Closed
timdiggins opened this issue Jan 7, 2024 · 8 comments

Comments

@timdiggins
Copy link

Background:
The quality of discussion on the PRs in this repo is really top notch and I've learned a lot from reading, e.g. #63 and #61. I initially started looking for the PRs because I couldn't work out what the point of Capybara/RSpec/HaveSelector was. The discussion on #63 was really great. I agree with the comment in that PR that the cops could do with a rationale - either stated within the docs of the cop, or as a Reference. Rubocop's cops mostly have references to the ruby style guide, which is where they draw their rationale.

I notice that the ["Reference")(https://github.com/rubocop/rubocop-capybara/blob/main/config/default.yml#L21) in rubocop-capybara is mostly a reference to itself. I propose that the References should be replaced with the best justification page for them (which in many cases will be the PR which introduced them).

FYI I believe (based on https://github.com/rubocop/rubocop/blob/2fe4b1a6faca23adff13e1bbff6ecf5b66c6447b/lib/rubocop/cop/message_annotator.rb#L102) there can be multiple Urls in the Reference

I think this would massively add to the value of rubocop-capybara. I will make a PR with a couple of starting points and see what people think.

timdiggins added a commit to timdiggins/rubocop-capybara that referenced this issue Jan 7, 2024
@timdiggins
Copy link
Author

main...timdiggins:rubocop-capybara:update-references-for-two-cops

But - it seems part of the build process for the gem actually replaces these references with the rubydoc.info URL. I'm not sure the justification for that (and it might be quite a bit of work to change) so I'll wait for feedback before progressing / making a PR.

@pirj
Copy link
Member

pirj commented Jan 8, 2024

@timdiggins thanks for bringing this up.
What needs to be done is really a style guide repo, rubocop/capybara-style-guide.
You may have noticed StyleGuideBaseURL in that class.

Here’s an example for rubocop-rspec

Please follow this thread for the guide details. Cc @ydah @mvz.

@bquorning should we create the right styleguide repo in the rubocop namespace from the start, or is it fine if @timdiggins starts a repo, and we transfer it after?

@bquorning
Copy link
Contributor

bquorning commented Jan 8, 2024

@pirj both solutions work for me. We’d have to set up a https://github.com/rubocop/capybara-style-guide repo, and have it publish into https://capybara.rubystyle.guide/.

Creating a separate repo, and then transfering it makes authentication/permissions easier to start with. Update: I created https://github.com/rubocop/capybara-style-guide, adding @timdiggins with write access and @rubocop/rubocop-rspec as admin.

@timdiggins
Copy link
Author

This is a much bigger proposition than just changing the Reference URLs of a the rules to the initial PRs! I'm not a capybara best practices expert, but I'm happy to start the ball rolling on a capybara guide. I've created a skeleton README.adoc, but am having trouble pushing to the repo (maybe because it's empty?) and can't fork (because it's empty).

Have put my proposed skeleton doc into rubocop/capybara-style-guide#1

@timdiggins
Copy link
Author

Oops - pushed the wrong branch first. Can someone with admin rights change the default branch back to main? I have put very uncontroversial parts of README in main. Then I can create PR for the initial skeleton wording (currently on https://github.com/rubocop/capybara-style-guide/tree/initial-proposal-for-structure)

(I worked out why I couldn't push, because I hadn't accepted invitation).

@bquorning
Copy link
Contributor

Can someone with admin rights change the default branch back to main?

Done

@ydah
Copy link
Member

ydah commented Jan 15, 2024

The following repositories have been created, so let's close this issue.
https://github.com/rubocop/capybara-style-guide

@ydah ydah closed this as completed Jan 15, 2024
@pirj
Copy link
Member

pirj commented Jan 15, 2024

What you did is just awesome! Thanks a lot for making this happen, guys

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

No branches or pull requests

4 participants