Skip to content

Conversation

@AaronMoat
Copy link
Contributor

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

Resolves #325 by running the existing logic inside tableCell, plus tests

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Mar 31, 2025
@codecov
Copy link

codecov bot commented Mar 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (802f78f) to head (d6b9b12).
Report is 80 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##              main      #326     +/-   ##
===========================================
  Coverage   100.00%   100.00%             
===========================================
  Files           72        88     +16     
  Lines        14745     21184   +6439     
===========================================
+ Hits         14745     21184   +6439     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AaronMoat AaronMoat marked this pull request as ready for review March 31, 2025 09:54
Copy link
Member

@JounQin JounQin left a comment

Choose a reason for hiding this comment

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

LGTM

```text
1:16-1:26: Unexpected reference to undefined definition, expected corresponding definition (`header 2`) for a link or escaped opening bracket (`\[`) for regular text
3:16-3:21: Unexpected reference to undefined definition, expected corresponding definition (`bar`) for a link or escaped opening bracket (`\[`) for regular text
```
Copy link
Member

Choose a reason for hiding this comment

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

Docs and tests are generated from the code.
The docs you wrote will be automatically changed.
You can remove the manual test.
And turn this, what you added manually in the docs, into an actual test, see the code: at the top of the index.js file, there are examples.

Please then run the npm scripts provided by this package. Thank you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, didn't realise this. I did think the coverage of the rule was low, makes sense that the examples (docs) are the tests! Fixed in 5fa9f25.

Copy link
Member

@wooorm wooorm left a comment

Choose a reason for hiding this comment

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

Nice! 2 comments:

*
* [Neptune][neptune][more] is the eighth and farthest planet from the Sun.
*
* - [Pluto], once considered the ninth planet, is now classified as a [dwarf planet][Pluto].
Copy link
Member

Choose a reason for hiding this comment

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

Is this change also needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've undone it. I was trying to cover lists, but probably unnecessary

Comment on lines 157 to 165
* | [Planet] | [Radius] |
* |---------------------|------------|
* | [Mercury] | 2,439.7 km |
* | [Venus][venus-docs] | 6,051.8 km |
* | [Earth] | 6,378 km |
*
* [Planet]: https://example.com/planet/
* [Mercury]: https://example.com/mercury/
* [Venus]: https://example.com/venus/
Copy link
Member

Choose a reason for hiding this comment

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

All other tests are as minimal as can be. Is it needed to be this big, this many references? Or can it be smaller. Such as:

Suggested change
* | [Planet] | [Radius] |
* |---------------------|------------|
* | [Mercury] | 2,439.7 km |
* | [Venus][venus-docs] | 6,051.8 km |
* | [Earth] | 6,378 km |
*
* [Planet]: https://example.com/planet/
* [Mercury]: https://example.com/mercury/
* [Venus]: https://example.com/venus/
* | [Planet] | [Radius] |
* | ----------------- | --------- |
* | Mercury | 2439.7 km |
*
* [planet]: https://example.com/planet/

(Also some small changes such as padding and no casing in definition ID, which are the more common used styles in these projects!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've simplified to almost this, with an extra link to Mercury to cover table cells (as well as headers)

@AaronMoat AaronMoat requested a review from wooorm April 3, 2025 22:57
@wooorm wooorm merged commit 3eb29f1 into remarkjs:main Apr 9, 2025
6 checks passed
@github-actions

This comment has been minimized.

@wooorm wooorm added the 💪 phase/solved Post is done label Apr 9, 2025
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Apr 9, 2025
@wooorm
Copy link
Member

wooorm commented Apr 9, 2025

Released in [email protected], thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💪 phase/solved Post is done

Development

Successfully merging this pull request may close these issues.

no-undefined-references: Missing support for tableCell

3 participants