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

feat: convert spelling check to a standard check targets #118

Merged
merged 17 commits into from
Dec 6, 2023

Conversation

apskhem
Copy link
Collaborator

@apskhem apskhem commented Dec 1, 2023

Description

Related Issue(s)

Closes #115

Description of Changes

Make spelling check conform to check target pattern

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@apskhem apskhem self-assigned this Dec 1, 2023
@apskhem apskhem linked an issue Dec 1, 2023 that may be closed by this pull request
@apskhem apskhem requested a review from stevenj December 1, 2023 16:47
@apskhem apskhem added the ci/cd CI/CD Fixes or Improvements. label Dec 1, 2023
@apskhem apskhem added the draft Review only, do not merge yet label Dec 4, 2023
@apskhem apskhem requested a review from bkioshn December 4, 2023 17:44
@apskhem apskhem removed the draft Review only, do not merge yet label Dec 4, 2023
docs/src/guides/spelling.md Outdated Show resolved Hide resolved
Earthfile Show resolved Hide resolved
Earthfile Show resolved Hide resolved
docs/src/guides/spelling.md Outdated Show resolved Hide resolved
earthly/cspell/Earthfile Show resolved Hide resolved
@Mr-Leshiy
Copy link
Contributor

BTW, I think we should add this check target to the separate Earthfile, for example to the earthly/cspell/Earthfile.
So the idea is to run spell checks and markdown checks separately in different job, rather than in one as we will put it under the similar check target inside the root Earthfile.
Because if we will put it under the same target like this

check:
    BUILD +check-spelling
    BUILD +check-markdown

if spell check will fail, markdown check will not even run, what we don't want

@stevenj

earthly/cspell/Readme.md Outdated Show resolved Hide resolved
@apskhem apskhem requested review from stevenj and Mr-Leshiy December 6, 2023 15:14
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

LGTM

@stevenj stevenj dismissed Mr-Leshiy’s stale review December 6, 2023 15:33

Will be resolved in a follow up task

@stevenj stevenj merged commit f888a89 into master Dec 6, 2023
46 checks passed
@stevenj stevenj deleted the feat-spell-check branch December 6, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/cd CI/CD Fixes or Improvements.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

🛠️ [TASK] : Convert Spelling check to a standard check targets.
3 participants