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

Ignore items where the checkbox is part of the strike-through #1

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

msfroh
Copy link

@msfroh msfroh commented Dec 1, 2023

I edit checklist items by striking out the whole line (after the bullet), so the checkbox is also part of the strike-through.

It took me several tries before reading the source code for this action to I understand that I must only strike through the words, not the box.

I edit checklist items by striking out the whole line (after the
bullet), so the checkbox is also part of the strike-through.

It took me several tries before reading the source code for this action
to I understand that I must only strike through the words, not the box.

Signed-off-by: Michael Froh <froh@amazon.com>
Copy link

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

+1

@dbwiddis
Copy link

Agreed we need some flexibility here.

  1. The error message is confusing and doesn't tell me clearly what I need to fix:
Check list item that should be checked `[x]` or the item should be 'struck out' with `~`
characters on both sides of the text. Line `~- [ ] New functionality includes testing.~
  1. The current text-only implementation leaves the box un-checked which gives inaccurate indications in the summary that the checklist has not been completed (see "1 of 8 tasks"):
    Screenshot 2024-02-19 at 10 58 38 PM

  2. This whole script really seems overbearing when we have automation.

    • I can self-attest that all tests pass, but you can just look at the list of failed tests and see that I'm lying
    • We have automated code coverage that should fail if tests weren't written
    • New functionality isn't really clearly defined, and the documentation sub bullet implies it's just javadocs but there are separate documentation bullets later. But there are tests for javadocs that we can easily add to enforce that part.

Copy link
Owner

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Thanks @msfroh and @dbwiddis

@peternied peternied merged commit ec79db6 into peternied:main Feb 20, 2024
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.

3 participants