Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Rewrite #15

Merged
merged 4 commits into from
Aug 28, 2015
Merged

Rewrite #15

merged 4 commits into from
Aug 28, 2015

Conversation

1000ch
Copy link
Member

@1000ch 1000ch commented Jul 27, 2015

@steelbrain
Copy link
Contributor

@iam4x wanna take this one too?

@steelbrain
Copy link
Contributor

@1000ch just a side note, but an important one, Atom, like any other chrome-based app has a single thread for rendering, and doing cpu-intensive operations on that process can significantly lag the editor for the user. I would recommend you jsonhint logic to another file called cli.js and spawn that file from this process using atom-linter's execNode.

@1000ch
Copy link
Member Author

1000ch commented Jul 28, 2015

@steelbrain Ah, I understand. But it will be unwillingly :O

@iam4x
Copy link
Member

iam4x commented Jul 28, 2015

@steelbrain Your statement is true, but it depends a lot. For instance we had to rollback linter-eslint to use an API because it feels much faster than spawning a process child. I think the process is almost the same for linter-json.

I will take this also.

@iam4x iam4x self-assigned this Jul 28, 2015
let column = 0;

return [{
type: 'error',
Copy link
Contributor

Choose a reason for hiding this comment

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

How about you uppercase this to Error

@steelbrain
Copy link
Contributor

@iam4x Great, I'll leave the rest to you!

@keplersj
Copy link
Contributor

The code looks good to me, on initial review. I can merge and publish if you say it's ready @1000ch.

This would fix #17

@1000ch
Copy link
Member Author

1000ch commented Aug 28, 2015

Please!

keplersj added a commit that referenced this pull request Aug 28, 2015
Rewritten for the new Linter API.

Fixes #17 
Fixes #16 
Fixes #14 
Fixes #13
@keplersj keplersj merged commit 24ab6cf into AtomLinter:master Aug 28, 2015
@keplersj
Copy link
Contributor

This will be the next major release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants