Skip to content

Fix CI vale check reporting and resolve vale errors#1278

Open
gilangjavier wants to merge 2 commits intocollective:mainfrom
gilangjavier:fix/issue-1277
Open

Fix CI vale check reporting and resolve vale errors#1278
gilangjavier wants to merge 2 commits intocollective:mainfrom
gilangjavier:fix/issue-1277

Conversation

@gilangjavier
Copy link

@gilangjavier gilangjavier commented Mar 15, 2026

This PR fixes the CI to correctly report a failure when make vale returns an error code by removing the shell conditional in the Makefile that was swallowing the exit status. It also resolves existing Vale errors:

  • Corrected misspelling of 'iCalendar' in docs/reference/component-api.rst.
  • Fixed punctuation placement in docs/contribute/maintenance.rst.
  • Added 'Repology' to the Vale vocabulary to allow it in docs/how-to/install.rst.
  • Standardized Sphinx configuration snippets in docs/contribute/maintenance.rst to use trailing commas for consistency.

📚 Documentation preview 📚: https://icalendar--1278.org.readthedocs.build/

Copy link
Member

Choose a reason for hiding this comment

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

Please revert all changes to this file. They're not correct, and they're irrelevant to the issue.

Args
backported
(?i)datetime
Repology
Copy link
Member

Choose a reason for hiding this comment

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

We first need to show that changes to the CI resolve the false positive. Please delete this change for now so it can be tested, and once the CI check does in fact fail, then it may be added and sorted alphabetically so it can pass. Does that make sense?

echo "For guidance of how to correct the errors, see:"; \
echo "https://icalendar.readthedocs.io/en/latest/contribute/documentation/build-check.html#spelling-grammar-and-style"; \
fi
@uv run vale --no-wrap $(VALEOPTS) $(VALEFILES)
Copy link
Member

@stevepiercy stevepiercy Mar 15, 2026

Choose a reason for hiding this comment

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

This change doesn't resolve the false positive.

This change purges a helpful message. Please don't remove it.

If the logic captures, but doesn't return the correct exit code, then the commands should be modified to return it.

@gilangjavier
Copy link
Author

I've added the missing changelog entry in CHANGES.rst. The changelog verifier is now green.

@stevepiercy
Copy link
Member

@gilangjavier thank you. However, we still need to see a failure in CI to verify that the changes actually do what we want. See #1278 (comment).

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.

2 participants