-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
New lint rule: Lesson overview items should not be questions. #27625
Comments
I agree with the idea here, though I think it makes more sense to be a style guide item than to try and enforce it with linting. |
@thatblindgeye Thoughts? I agree about consistency with lesson overview points not being questions (which also makes more sense for a lesson overview anyway). I'm more inclined to agree with Damon that just having it in the lesson style guide is sufficient, and just rewording any that currently exist as questions to be statements instead, as opposed to making an entire rule for it, which again would only check for |
FWIW there was an intent that updating the original section ("Learning outcomes") to "Lesson overview" would include updating the items from questions ("What is XYZ?") to statements ("What XYZ is"). If it's not already part of the style guide it should be added on, which this issue could be converted to doing that if anything. As a custom rule... I do think that we should probably try to have as many rules that cover what our style guide is conveying as we can. Even if the best way to check whether an LO item is a question is by checking for a "?" at the end, it's not too dissimilar to our rule that checks for inline code in headings. Obviously the difference there is it's pretty difficult to sneak an inline code by, unlike a question being able to omit the question mark ("What is XYZ" or "What is XYZ."). So I think I'm actually of a differing opinion, in that it may be worth having a custom rule for this. It very much depends on ensuring that it catches more instances than it misses, and that false-positives are kept to a minimum or can easily be fixed by rewording the LO item. For example, maybe something like "What is XYZ and how to implement it" is valid, but could (should?) be reworded as "What XYZ is and how to implement it." Maybe we also enforce that LO items must be worded as a sentence with proper punctuation, i.e. a period. @MaoShizhong if you wouldn't be totally opposed I'd actually like to see what @nikitarevenco could come up with, keeping in mind that it may end up being something we do decide doesn't work as well as a rule. |
If Revenco would still like to pursue this, I have no objections then. I've some ideas regarding more than just matching |
@nikitarevenco would you still like to be assigned? |
Yes, I'm happy to work on this! |
This issue is stale because it has had no activity for the last 30 days. |
@nikitarevenco just wanted to check in on the status of this issue. |
I've made the draft PR but I wasn't sure how I can actually test this so I was mostly in the fog when making this. I now realise I should've asked but oh well. I added docs, tests and code for the rule but I don't know how I can test the code without making the PR And I was expecting the automatic checks to run on the PR but it seems like they aren't |
Going to reply in the draft PR |
Checks
Describe your suggestion
I don't think it makes sense for the bullet points to be questions, they should be instead be statements about what the learner will learn during the lesson
So instead of this:
Lesson overview
This section contains a general overview of topics that you will learn in this lesson.
The following may be better:
Lesson overview
This section contains a general overview of topics that you will learn in this lesson.
Path
Foundations, Ruby / Rails, Node / JS
Lesson Url
Linter
(Optional) Discord Name
Revenco
(Optional) Additional Comments
No response
The text was updated successfully, but these errors were encountered: