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

[UPDATE] Updated project to 0.19.1 #38

Merged
merged 2 commits into from
Jan 18, 2016
Merged

[UPDATE] Updated project to 0.19.1 #38

merged 2 commits into from
Jan 18, 2016

Conversation

NullDivision
Copy link
Contributor

  • added some ignores to flow from node_modules
  • updated Babel to version 6
  • fixed kill function so it kills even if no args are present
  • added message when the next message is part of current file
  • separated lint functions
  • fixed test failing on new version
  • latest stable versions installed for all packages

- added some ignores to flow from node_modules
- updated Babel to version 6
- fixed kill function so it kills even if no args are present
- added message when the next message is part of current file
- separated lint functions
- fixed test failing on new version
- latest stable versions installed for all packages
var flowToJshint = require('flow-to-jshint');
var stylishReporter = require(require('jshint-stylish')).reporter;
var stylishReporter = require('jshint-stylish').reporter;
Copy link
Collaborator

Choose a reason for hiding this comment

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

'require' is not defined.

@NullDivision NullDivision mentioned this pull request Dec 10, 2015
@NullDivision
Copy link
Contributor Author

I found an issue with default options not being translated so please hold off on this request. Sorry.

@NullDivision
Copy link
Contributor Author

Everything should be working correctly.

Issues resolved:

  • updated to version 1.19.1 (issue flow-bin version #35)
  • process not being killed with killFlow option active
  • tests failing
  • double-line messages not displaying correctly

@charliedowler
Copy link
Collaborator

Hi,

Thanks a lot for doing this, much appreciated!

It all looks good to me except from the multiline errors.

Output from original code:
original code PR output
Output from your PR:
NullDivision PR output

My flow-to-jshint splits new lines and splices them into a single line separated by '-' as I feel it looks much neater.

Cheers,
Charlie

@NullDivision
Copy link
Contributor Author

I did not know that :) I'll have to take a look at the docs for flow-to-jshint but this is a change in the new format of Flow itself. I'll see what I can do about the multiple lines.

@NullDivision
Copy link
Contributor Author

So, the issue is that the new implementation (because I'm not sure of the old one) is that errors are no longer in a single message but in multiple messages per error. flow-to-jshint only changes new lines to - separation.

As this is no longer the case would it be possible to merge with the new implementation and then change it to single line if that's what you want?

return error.message.length > 0;
let lastFile = '';
let isCurrentFile = error.message[0].path === _path;
let generalError = (/(Fatal)/.test(error.message[0].descr));
Copy link
Collaborator

Choose a reason for hiding this comment

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

'let' is available in ES6 (use esnext option) or Mozilla JS extensions (use moz).

@NullDivision
Copy link
Contributor Author

While going through documentation and various tests here's what I discovered:

  • each file can have multiple errors
  • each error can have one or more messages
  • the first message contains the file being linted
  • subsequent messages have a path only if referencing (eg. [{desc: 'variable X', path: 'file.js'}, {desc: 'incompatible with type', file: ''}, {desc: 'Y', path: 'some/other/file.js'}])

As such I removed the whole block that tests next and previous messages because all 3 messages together make up the context as a whole, otherwise you'd end up with an error like Variable X incompatible with or lib instead of lib Required module not found.

- messages restructured in flow
- update to Flow 0.20.1
- added stage 2 for babel
@NullDivision
Copy link
Contributor Author

Oh, and I also incremented to version 0.49.0 and updated Flow to 0.20.1.

charliedowler added a commit that referenced this pull request Jan 18, 2016
[UPDATE] Updated project to 0.19.1
@charliedowler charliedowler merged commit c916e2a into stcheng:master Jan 18, 2016
@charliedowler
Copy link
Collaborator

Hi,

Thanks a lot for the PR, it should now be published.

My internet dropped out while publishing so I am not sure if it worked, tried to re-publish but can't overwrite the version number.

Let me know if you have any issues with 0.4.9, I'll try again if needed.

Cheers,
Charlie

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