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

Analyze Build Redux #131

Open
ajmueller opened this issue Apr 4, 2015 · 7 comments
Open

Analyze Build Redux #131

ajmueller opened this issue Apr 4, 2015 · 7 comments

Comments

@ajmueller
Copy link
Contributor

Before the current iteration of the build system was built by @kamsar, we had an "analyze" build. That build went the way of the Dodo due to the driver architecture of the build process (so @kamsar if this idea is totally impossible, let me know), but I think it may be worth discussing again considering our recent internal discussions around practice groups and standards. Those standards could be more easily enforced with a build process for analysis, especially during code reviews. Some ideas and packages to use:

Thoughts?

@kamsar
Copy link
Contributor

kamsar commented Apr 4, 2015

tbh I think any sort of analysis is a bit toothless as long as it's something optional you have to run (which is why it was removed during the build update).

IMO, if analysis is brought in it should run as part of the build and fail the build if analysis finds problems. Yes, you may have to disable some analysis features you dont agree with. But the old analyze build was never used.

The scripts should support this idea fairly easily.

@jbascue
Copy link

jbascue commented Apr 5, 2015

That model works for Visual Studio.

I miss the analysis and liked where Alex was going with the idea of
bringing it back, but I really like the idea of it being integrated right
into the build like this a lot. It enforces standards that much better.
On Sat, Apr 4, 2015 at 1:48 PM Kam Figy notifications@github.com wrote:

tbh I think any sort of analysis is a bit toothless as long as it's
something optional you have to run (which is why it was removed during the
build update).

IMO, if analysis is brought in it should run as part of the build and
fail the build if analysis finds problems. Yes, you may have to disable
some analysis features you dont agree with. But the old analyze build was
never used.

The scripts should support this idea fairly easily.


Reply to this email directly or view it on GitHub
#131 (comment)
.

@stoff
Copy link
Contributor

stoff commented Apr 8, 2015

Just would like to add to the chorus of "should be part of the build", however, we'd want to look a little more closely at annoyances this may cause in a watch build.

Specifically I'm thinking of times when an unnoticed JS error has stopped my watch and I don't notice until after I've already noticed the bad save, fixed the issue, and then refreshed my browser (only to discover, shock!, that my changes don't appear or that the css is not reloading)

Also useful might be writing a log since there are times when the error report may be longer than the user's terminal buffer.

@ajmueller
Copy link
Contributor Author

👍 for writing to a log. That was one feature about our previous analyze build that I wasn't a fan of: only console output.

@kamsar
Copy link
Contributor

kamsar commented Apr 9, 2015

$ build-dev.command > build.log :)

Saucy console tricks aside, if your error list is really longer than your terminal buffer uh...that's a problem.

@ajmueller
Copy link
Contributor Author

Ooooh, nice. That would seemingly be easy to incorporate into the shell scripts then, right? I personally find it nicer to read a file in a text editor than a console window, so having a consistent log would be nice.

@kamsar
Copy link
Contributor

kamsar commented Apr 9, 2015

http://en.wikipedia.org/wiki/Tee_%28command%29 would work to send to console AND log
Some of the validators may also have built in file logging options.

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

No branches or pull requests

5 participants