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 some more flow types #3186

Merged
merged 4 commits into from Feb 4, 2016
Merged

add some more flow types #3186

merged 4 commits into from Feb 4, 2016

Conversation

hzoo
Copy link
Member

@hzoo hzoo commented Dec 18, 2015

Also revert #3275

@amasad
Copy link
Member

amasad commented Dec 23, 2015

failing

@hzoo hzoo force-pushed the add-some-flow branch 3 times, most recently from fb8aa0e to ec7b9e0 Compare December 31, 2015 17:09
@hzoo
Copy link
Member Author

hzoo commented Jan 1, 2016

This is only passing on node 5

packages/babel-code-frame/src/index.js:6
  6: import esutils from "esutils";
                         ^^^^^^^^^ esutils. Required module not found

so i'm assuming this is an issue with npm 2?

@hzoo hzoo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Jan 18, 2016
@hzoo hzoo force-pushed the add-some-flow branch 5 times, most recently from dcda4b5 to 6ee3aca Compare February 2, 2016 23:48
@codecov-io
Copy link

Current coverage is 85.05%

Merging #3186 into master will decrease coverage by -0.24% as of f3ceb1b

@@            master   #3186   diff @@
======================================
  Files          215     215       
  Stmts        15773   15773       
  Branches      3382    3382       
  Methods          0       0       
======================================
- Hit          13454   13416    -38
- Partial        688     725    +37
- Missed        1631    1632     +1

Review entire Coverage Diff as of f3ceb1b

Powered by Codecov. Updated on successful CI builds.

@amasad
Copy link
Member

amasad commented Feb 3, 2016

let me know when this is ready 💅

@hzoo
Copy link
Member Author

hzoo commented Feb 3, 2016

I suppose it is (since it finally passes)? I dono what is going on with ^ esutils. Required module not found - it sounds like it has to do with how flow does require statements and something with npm 2 since it works in node 5 / npm 3. I had to remove flowcheck from those 2 files

@amasad
Copy link
Member

amasad commented Feb 3, 2016

Is it hard to update the CI to node 5 / npm 3?

@hzoo
Copy link
Member Author

hzoo commented Feb 3, 2016

What do you mean? https://github.com/babel/babel/blob/master/.travis.yml our travis has 0.10, etc which are all npm 2. Do you mean doing something like npm install npm@3 -g for the older versions?

@amasad
Copy link
Member

amasad commented Feb 3, 2016

That or maybe just run flow on node 5 environments:

node_v=`node -v`
if [ "$node_v" = "v5.5.0" ]; then flow; fi

or something like that

@hzoo
Copy link
Member Author

hzoo commented Feb 3, 2016

Ooh something like that sounds good actually. Can just check node > 5

@amasad
Copy link
Member

amasad commented Feb 3, 2016

Yeah needs a bit more bash fu than I have to parse the int but it's doable.

@hzoo
Copy link
Member Author

hzoo commented Feb 3, 2016

time to google (or I just write it in js and call the script).

@amasad
Copy link
Member

amasad commented Feb 3, 2016

Lol why not

@hzoo hzoo force-pushed the add-some-flow branch 2 times, most recently from 04a8f0d to aa307f9 Compare February 3, 2016 02:18
@hzoo
Copy link
Member Author

hzoo commented Feb 3, 2016

Ok probably lazy/unnecessary but it works.

@amasad
Copy link
Member

amasad commented Feb 3, 2016

Cool, do you want to revert the noflow checks you had to add?

@hzoo
Copy link
Member Author

hzoo commented Feb 3, 2016

Yeah I did already 😎

@hzoo
Copy link
Member Author

hzoo commented Feb 3, 2016

Although I think I can remove some more

@hzoo hzoo force-pushed the add-some-flow branch 3 times, most recently from a07b86e to 640a80b Compare February 4, 2016 15:17
hzoo added a commit that referenced this pull request Feb 4, 2016
@hzoo hzoo merged commit 807e190 into babel:master Feb 4, 2016
@hzoo hzoo deleted the add-some-flow branch February 25, 2017 17:29
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants