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

Introduce text checks (vale.sh) on Docstrings and comments #353

Merged
merged 56 commits into from
Feb 15, 2024

Conversation

kellertuer
Copy link
Member

This PR slowly works through all doc strings to fulfil the vale.sh rules the documentation already follows, because since the 3.0 version vale.sh works on Julia files and recognises its doc-strings.

# Conflicts:
#	docs/make.jl
#	ext/ManoptManifoldsExt/manifold_functions.jl
# Conflicts:
#	test/helpers/test_manifold_extra_functions.jl
# Conflicts:
#	src/helpers/checks.jl
#	test/solvers/test_particle_swarm.jl
@kellertuer kellertuer added the WIP Work in Progress (for a pull request) label Jan 31, 2024
Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8851619) 99.45% compared to head (7a72d41) 99.22%.
Report is 4 commits behind head on master.

❗ Current head 7a72d41 differs from pull request most recent head 1f0180b. Consider uploading reports for the commit 1f0180b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #353      +/-   ##
==========================================
- Coverage   99.45%   99.22%   -0.23%     
==========================================
  Files          69       69              
  Lines        6418     6194     -224     
==========================================
- Hits         6383     6146     -237     
- Misses         35       48      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kellertuer and others added 12 commits February 4, 2024 17:45
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
# Conflicts:
#	src/plans/debug.jl
#	src/plans/plan.jl
#	test/plans/test_stopping_criteria.jl
#	tutorials/Optimize.qmd
# Conflicts:
#	src/solvers/quasi_Newton.jl
#	src/solvers/subgradient.jl
@kellertuer kellertuer marked this pull request as ready for review February 15, 2024 07:53
@kellertuer
Copy link
Member Author

Vale does not yet fully work, so I do not want to activate a CI on this yet. The main challenges are

  • maybe I was a bit to eager declaring strings to be vale-d as well (especially when they are formats)
  • qmd files and their rendered results are not handled correctly, code in those markdown files seems to be treated as text, which of course is not what it is and complains about _ that should not be used in words or such.

Still. These first steps

  • improve readability
  • have a large scale change, so I do not want to keep this PR open longer.

Since this does not add new features, but just rewrites docs, this will not be a new version, but be on master until a next feature justifying a new version comes along.

.codecov.yml Outdated Show resolved Hide resolved
@mateuszbaran
Copy link
Member

  • maybe I was a bit to eager declaring strings to be vale-d as well (especially when they are formats)
  • qmd files and their rendered results are not handled correctly, code in those markdown files seems to be treated as text, which of course is not what it is and complains about _ that should not be used in words or such.

Would it be possible to enable Vale on CI only for comments but not for strings and qmd files?

BTW, I most likely won't look over all these changes so merge when you think it's ready.

@kellertuer
Copy link
Member Author

Sure it is a lot of changes, no worries – and most of them are even mainly unifying formatting of Keyword arguments and such.

For comments and not strings – in theory yes, in practice, I can not find how to do that.
For dmd files I for now deactivated that, but locally it still vales the resulting *.md files as well and I am not able to say “please do not check this folder, those are autogenerated anyways”. As far as I could read about it, vale is not meant to ignore folders by design or something.

Co-authored-by: Mateusz Baran <[email protected]>
@kellertuer kellertuer added documentation Ready-for-Review A label for pull requests that are feature-ready and removed WIP Work in Progress (for a pull request) documentation Ready-for-Review A label for pull requests that are feature-ready labels Feb 15, 2024
@kellertuer kellertuer merged commit ca6f2de into master Feb 15, 2024
13 checks passed
@kellertuer kellertuer deleted the kellertuer/vale-on-code branch May 4, 2024 17:30
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