dciy icon indicating copy to clipboard operation
dciy copied to clipboard

Validate empty repo

Open smashwilson opened this issue 10 years ago • 4 comments

This addresses #23 by:

  • Adding validation to the Project model.
  • Fixing a bug in ProjectsController that only shows up when validation fails :wink:
  • Reporting the output of git fetch or git clone when either fail, and failing the build then too.

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 avatar Jan 17 '14 19:01 smashwilson

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.

smashwilson avatar Jan 17 '14 20:01 smashwilson

:+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.

cobyism avatar Jan 20 '14 20:01 cobyism

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.

smashwilson avatar Jan 21 '14 15:01 smashwilson

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.

cobyism avatar Jan 21 '14 16:01 cobyism