David Cornu davidcornu

Organizations

@Shopify @railsbridge-montreal
@davidcornu

ci/circleci is ignored from the build results, so we're left with a single green status Ah. So we should probably show that visually to avoid con…

@davidcornu

If all the builds aren't complete, the build status should not be a green checkbox

@davidcornu
Build states are wonky
@davidcornu

Can we use Task#user or is that set based on the commit?

davidcornu commented on pull request Shopify/shipit-engine#464
@davidcornu

I can't recall of any lock that would cause a major issue if it was lost. Some jobs would fail, but the application would come back a a proper sta…

davidcornu commented on pull request ryanb/letter_opener#109
@davidcornu

@fw42 minor comment, otherwise :+1:

davidcornu commented on pull request ryanb/letter_opener#109
@davidcornu

I believe you can just do defined?(Rails::Railtie) instead of checking for Rails first

davidcornu commented on pull request rails/rails#20761
@davidcornu

It doesn't seem unusual for an AV helper to assume you want HTML output @matthewd except in this case the t helper is supposed to escape all outp…

davidcornu commented on pull request rails/rails#18909
@davidcornu

This may have been addressed by @sirupsen's PR #20828. Will have to test.

davidcornu commented on pull request Shopify/shipit-engine#453
@davidcornu

@KnVerey also suggested visually making one of them the primary action and the other a secondary one.

davidcornu commented on pull request Shopify/shipit-engine#451
@davidcornu

I also just had the idea that it might be useful to have one of the buttons look "primary" to help someone underinformed quickly make the right de…

davidcornu commented on pull request Shopify/shipit-engine#451
@davidcornu

@byroot any design :eyes: we can summon?

davidcornu commented on pull request Shopify/shipit-engine#451
@davidcornu

I updated the JS in regards to @davidcornu's comments :+1: and it also now hide the other button during the recovery period to avoid confusion. S…

@davidcornu
Sticky header on the deploy page causes jumpiness
davidcornu commented on pull request Shopify/shipit-engine#451
@davidcornu

Those buttons look like they're floating about IMHO. Not entirely sure where they should live though.

davidcornu commented on pull request Shopify/shipit-engine#451
@davidcornu

Seems odd to me to build an AbortButton by passing AbortButton.handle as a callback. IMHO having something like AbortButton.listen() handle the bin…

davidcornu commented on pull request Shopify/shipit-engine#451
@davidcornu

Is the .closest necessary given that you're binding to a click event on [data-action="abort"]?

@davidcornu

Should we do this in JS? Would avoid additional rails logic for a presentation concern. Either way is fine though.

@davidcornu

Could we do this in js?

@davidcornu

The rest of the form needs some love too. IMHO the help text looks like a misplaced label and the checkbox for skipping CI should be on the other …

davidcornu commented on pull request rails/rails#20761
@davidcornu

@rafaelfranca updated version would return translation missing: key for non html-safe keys so it's still pretty visible. Also, they can always set A…

@davidcornu
davidcornu commented on pull request rails/rails#20761
davidcornu commented on pull request Shopify/shipit-engine#447
@davidcornu

I've restricted it to deploy:stack. Since custom tasks can possibly do anything, I think deploying, rolling back and running a custom task should …

davidcornu commented on pull request rails/rails#20761
@davidcornu

Also updated the documentation to reflect the new behaviour. I changed the order a little so that the concept of html-safe keys was discussed befor…

@davidcornu
  • @davidcornu 065640e
    Update documentation for TranslationHelper#translate
davidcornu commented on pull request rails/rails#20761
@davidcornu

Just pushed a fix for the broken tests

@davidcornu
  • @davidcornu 26b475a
    Return a plaintext missing translation message for non-html keys