Hi @quietone Just starting out and trying to learn how to contribute. Thank you!
- 🇮🇳India ankitv18
Reviewed the MR and changes made are clearer and understandable in the statement and usage of cspell-ignore and disable is at appropriate place.
This looks good to be merged, hence marking this RTBC - 🇮🇳India ankitv18
Considering the CSpell pipeline changes looks fine to be merged ~~ Marking this RTBC
- 🇳🇿New Zealand quietone New Zealand
@sanket.tale, Welcome to Drupal! Take care when creating forks, this is a meta issue that does not need an MR. I urge you to read the Contribution Guide → to help you find issues suited to your skills and interests.
- 🇮🇳India onkararun
@quietone could you please explain the steps to reproduce this issue.
- @quietone opened merge request.
- Issue created by @quietone
- 🇺🇸United States smustgrave
Pinged the frontend managers on this today just for the record
- 🇺🇸United States smustgrave
Tested my theory and I was wrong so restored to what it was. LGTM
- 🇺🇸United States smustgrave
Super easy to review
the 10 items in the summary match the 10 removed in the MR
Checking the changes seems to be changing the key "testkey" which seems fine
Nothing stands out to me - First commit to issue fork.
- 🇳🇿New Zealand quietone New Zealand
I think instead of figuring out why cspell has a word or not in a particular dictionary we should focus on what works for Drupal. Do we a) change the code/comments or b) add a given word to the drupal dictionary or c) make a PR to have it added to the a given dictionary.
Of those choices, what should happen to the 4 words in this issue?
The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇳🇿New Zealand quietone New Zealand
The method \Drupal\Tests\StreamCapturer::filter has two "// cSpell:disable-next-line" lines. While I usually follow the existing pattern in a file adding another // cSpell:disable-next-line" in a method with 6 lines of code just add clutter. Therefor, I moved all the words to a single ignore line.
- @quietone opened merge request.
- @quietone opened merge request.
- @quietone opened merge request.
- Issue created by @quietone