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

Add "fixme" as a valid purpose #30

Merged
merged 1 commit into from
Sep 16, 2020
Merged

Add "fixme" as a valid purpose #30

merged 1 commit into from
Sep 16, 2020

Conversation

jesstelford
Copy link
Contributor

@jesstelford jesstelford commented Sep 11, 2020

Brings the behaviour into line with the documentation:

Never forget a // TODO or // FIXME again.

@changeset-bot
Copy link

changeset-bot bot commented Sep 11, 2020

🦋 Changeset is good to go

Latest commit: 721e090

We got this.

This PR includes changesets to release 10 packages
Name Type
@markings/source-comments Minor
@markings/source-react Minor
@markings/types Minor
@markings/test-project Patch
@markings/cli Patch
@markings/output-csv Patch
@markings/output-html Patch
@markings/output-json Patch
@markings/react Patch
@markings/test-utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@emmatown emmatown left a comment

Choose a reason for hiding this comment

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

Could you make it so fixme comments have the purpose of fixme in https://github.com/Thinkmill/markings/blob/master/packages/source-comments/src/index.ts#L19 as well and revert the case insensitive check change?

re why case sensitivity for the prop: we control the Marking component, we can enforce how it works so let's just not have multiple ways to do the same thing.

(if you want to make the comment source case insensitive, that's fine imo because Markings can't really control what people think a todo comment is)

@jesstelford
Copy link
Contributor Author

jesstelford commented Sep 16, 2020

Have made those changes 👍

Will open a separate issue to discuss the case insensitivity. EDIT: #31

@emmatown emmatown merged commit bb5c58f into master Sep 16, 2020
@emmatown emmatown deleted the fixme-purpose branch September 16, 2020 03:03
@github-actions github-actions bot mentioned this pull request Sep 16, 2020
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