cobyism / dciy

Do Continuous Integration Yourself — a simple, open source CI server.
98 stars 10 forks source link

Validate empty repo #33

Open smashwilson opened 10 years ago

smashwilson commented 10 years ago

This addresses #23 by:

I also prevented git from holding up Sidekiq by asking for a password on stdin while I was at it, since I did some testing against private repos on GitHub.

smashwilson commented 10 years ago

The validation errors looks kind of ugly in the form. Think I should mess with the CSS on this PR too?

Oh, something else. When the build fails because of an exception, the stack trace pushes all of the nice messages off into the abyss of scrollback! I was thinking of adding an if in there to only show the full backtrace when:

  1. The exception inherits some set of known exceptions that we "expect" to show up, and
  2. You're not running in RAILS_ENV=production.

Of course the danger is that we'd make it harder to track down issues if people hit bugs in DCIY itself. Maybe we could log the full trace to @logger instead? In any case, that's definitely a separate issue.

cobyism commented 10 years ago

:+1: Awesome!

I also prevented git from holding up Sidekiq by asking for a password on stdin while I was at it

:heartpulse: I hadn’t run into that myself, but it’s great to have that sorted :grinning:

Think I should mess with the CSS on this PR too?

Only if you want to. I wouldn’t worry about merging this in even if the errors look horrible. It’s not that common a problem, and if someone using this is bothered enough about the errors not being pretty, they might be motivated enough to improve them themselves :bowtie:

When the build fails because of an exception, the stack trace pushes all of the nice messages off into the abyss of scrollback!

Another option might be to add a field/column to the Build model to separately capture and store any stacktraces that happen during a build? That way, the build output could simply say that it ran into an error, and we could display a separate output field below (maybe hidden/collapsed by default) containing the error stacktrace? I dunno… that could be a terrible idea for other reasons too. :laughing: Ideally it’d be cool if you could do "folding" in the output directly, but that seems like it’d take a fair bit of effort and time to even see if it’s viable.

smashwilson commented 10 years ago

Only if you want to. I wouldn’t worry about merging this in even if the errors look horrible. It’s not that common a problem, and if someone using this is bothered enough about the errors not being pretty, they might be motivated enough to improve them themselves :bowtie:

Sounds good to me! I might take a stab at it at some point if I'm feeling particularly artistic. :art:

Another option might be to add a field/column to the Build model to separately capture and store any stacktraces that happen during a build? That way, the build output could simply say that it ran into an error, and we could display a separate output field below (maybe hidden/collapsed by default) containing the error stacktrace? I dunno… that could be a terrible idea for other reasons too. :laughing: Ideally it’d be cool if you could do "folding" in the output directly, but that seems like it’d take a fair bit of effort and time to even see if it’s viable.

Oh, interesting! I like that as a way of dealing with "internal" problems. Keeps the build output clean, and lets us differentiate between DCIY problems and actual build problems at the same time. We could give exceptions like BranchNotFoundError "friendly" text explaining details and possible fixes, and show the full stack in a collapsed control.

Folding would be a lot of work, yeah; to get there we'd need to implement build output processing that can recognize what a stack trace looks like. It would be neat (if rather un-minimalistic) to be able to parse out RSpec, Test::Unit, or other testing frameworks' failure output and have special highlighting for the error messages. If you're looking at build output, that's what you're most interested in, after all! A logical extension is to recognize things like bundle install output and fold it up by default, Travis-style.

Anyway, this is all pretty speculative. I'll pop open a separate issue to improve exception handling, maybe.

cobyism commented 10 years ago

It would be neat (if rather un-minimalistic) to be able to parse out RSpec, Test::Unit, or other testing frameworks' failure output and have special highlighting for the error messages. If you're looking at build output, that's what you're most interested in, after all!

Yeah, that’d be :metal:—also, getting color support for build output would be pretty helpful too, in terms of highlighting what people care about while letting everything else fade into the background.