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

allow the use of double slash flow comments #17008

Merged
merged 1 commit into from May 29, 2018

Conversation

Projects
None yet
3 participants
@saadq
Contributor

saadq commented Mar 21, 2018

Description of the Change

Currently, a Flow comment needs to be used with the following syntax: /* @flow */. This PR allows the usage of // @flow as well.

Why Should This Be In Core?

To allow flexibility the same way that both 'use babel' and "use babel" are supported.

Benefits

It's a convenience.

Possible Drawbacks

It is kind of a trivial addition.

Verification Process

I wrote a test case for this, but I was having trouble getting npm test or script/test to work for me. I got an error like so:

AssertionError [ERR_ASSERTION]: More than one application to run tests against was found. 

I'm not sure how I'm supposed to develop on Atom if I already have Atom installed on my computer which I imagine is causing this error.

Applicable Issues

N/A

@lee-dohm

This comment has been minimized.

Member

lee-dohm commented Mar 21, 2018

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented May 29, 2018

Since tests (including the new one) pass and the change is very straightforward, I'm ok 🚢 this at this point. You cool with that @lee-dohm ?

@lee-dohm lee-dohm merged commit 57572d5 into atom:master May 29, 2018

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@lee-dohm

This comment has been minimized.

Member

lee-dohm commented May 29, 2018

Thanks for the contribution 👍

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