search

AtomLinter / linter-glsl

Atom package that lints GLSL shaders on the fly.
https://atom.io/packages/linter-glsl
MIT License
15 stars 6 forks source link

Update atom-linter to version 5.0.2 🚀 #35

Closed greenkeeperio-bot closed 8 years ago

greenkeeperio-bot commented 8 years ago

Hello lovely humans,

atom-linter just published its new version 5.0.2.

State Update :rocket:
Dependency atom-linter
New version 5.0.2
Type dependency

This version is not covered by your current version range.

Without accepting this pull request your project will work just like it did before. There might be a bunch of new features, fixes and perf improvements that the maintainers worked on for you though.

I recommend you look into these changes and try to get onto the latest version of atom-linter. Given that you have a decent test suite, a passing build is a strong indicator that you can take advantage of these changes by merging the proposed change into your project. Otherwise this branch is a great starting point for you to work on the update.

Do you have any ideas how I could improve these pull requests? Did I report anything you think isn’t right? Are you unsure about how things are supposed to work?

There is a collection of frequently asked questions and while I’m just a bot, there is a group of people who are happy to teach me new things. Let them know.

Good luck with your project :sparkles:

You rock!

:palm_tree:

The new version differs by 159 commits .

There are 159 commits in total. See the full diff.

This pull request was created by greenkeeper.io.

Tired of seeing this sponsor message? :zap: greenkeeper upgrade

Arcanemagus commented 8 years ago

@andystanton from what I can tell of trying to run this manually, the messages have changed, can you confirm and possibly see what is going on here?

andystanton commented 8 years ago

I'll take a look @Arcanemagus

andystanton commented 8 years ago

I caught the error and it looks like this:

[79911:0623/073148:INFO:CONSOLE(334)] "Error: Process exited with non-zero code: 2", source: /Users/andy/Code/linter-glsl/lib/linter-glsl.js (334)

I verified that the linting app (glslangValidator) does indeed exit with 2 when there are linting errors:

$ /usr/local/bin/glslangValidator /Users/andy/Code/linter-glsl/spec/fixtures/vert/sample.vert

ERROR: 0:2: 'main' : illegal use of type 'void'
ERROR: 0:2: '' :  syntax error
ERROR: 2 compilation errors.  No code generated.

$ echo $?

2

But the output goes to stdout:

$ /usr/local/bin/glslangValidator /Users/andy/Code/linter-glsl/spec/fixtures/vert/sample.vert 1>/dev/null

$ /usr/local/bin/glslangValidator /Users/andy/Code/linter-glsl/spec/fixtures/vert/sample.vert 2>/dev/null

ERROR: 0:2: 'main' : illegal use of type 'void'
ERROR: 0:2: '' :  syntax error
ERROR: 2 compilation errors.  No code generated.

Despite the linter exiting with 2, the messages are written to stdout. sb-exec states that if you're writing to stdout you must have had an exit code of 0, otherwise it rejects with an error message: https://github.com/steelbrain/exec/blob/master/src/index.js#L48-L50

The approach the glslangValidator team appear to have taken is that the exit code determines whether or not the linting passed, but that regardless of success or failure, the error messages are still part of the expected program output.

It can be argued that this approach isn't correct, but I also don't think it should be up to the executing library to mandate that reading from stdout implies an non-0 exit code. What are your thoughts @steelbrain?

steelbrain commented 8 years ago

@andystanton We have an ignoreExitCode option, you could use that

andystanton commented 8 years ago

Perfect, many thanks @steelbrain