|
|
Created:
8 years, 8 months ago by twifkak Modified:
8 years, 8 months ago CC:
codereview-list_googlegroups.com Visibility:
Public. |
DescriptionFix 'o' keyboard shortcut on dashboard page.
Patch Set 1 #
MessagesTotal messages: 15
This is related to this pull request: https://github.com/rietveld-codereview/rietveld/pull/526 It's not clear from the instructions at https://github.com/rietveld-codereview/rietveld/wiki/Contributing how to submit it after the issue is closed, so sorry if the pull request is a dupe. FWIW, firstElementChild is not supported on IE8 or lower, if that's a concern: https://developer.mozilla.org/en-US/docs/Web/API/ParentNode/firstElementChild. But right now the 'o' key doesn't work on _any_ browser, so this seems like a strict improvement.
Sign in to reply to this message.
On 2015/07/20 19:25:29, twifkak wrote: > This is related to this pull request: > https://github.com/rietveld-codereview/rietveld/pull/526 > > It's not clear from the instructions at > https://github.com/rietveld-codereview/rietveld/wiki/Contributing how to submit > it after the issue is closed, so sorry if the pull request is a dupe. > > FWIW, firstElementChild is not supported on IE8 or lower, if that's a concern: > https://developer.mozilla.org/en-US/docs/Web/API/ParentNode/firstElementChild. > But right now the 'o' key doesn't work on _any_ browser, so this seems like a > strict improvement. Email was eated. Trying again.
Sign in to reply to this message.
+albrecht.andi@gmail.com Andi: Ping. Is this the right place to email for code reviews?
Sign in to reply to this message.
+jrobbins (corp)
Sign in to reply to this message.
lgtm This change looks like a good fix. I'm not going to commit it or deploy it because I am no longer active in the original rietveld project, so you'll need help from someone who is.
Sign in to reply to this message.
On 2015/07/30 15:40:05, jrobbins (corp) wrote: > lgtm > > This change looks like a good fix. I'm not going to commit it or deploy it > because I am no longer active in the original rietveld project, so you'll need > help from someone who is. Oh, okay. Do you know who is?
Sign in to reply to this message.
I see that there is some activity on https://groups.google.com/forum/#!forum/codereview-discuss I think you were right to contact Andi. On Thu, Jul 30, 2015 at 9:17 AM, <twifkak@google.com> wrote: > On 2015/07/30 15:40:05, jrobbins (corp) wrote: > >> lgtm >> > > This change looks like a good fix. I'm not going to commit it or >> > deploy it > >> because I am no longer active in the original rietveld project, so >> > you'll need > >> help from someone who is. >> > > Oh, okay. Do you know who is? > > https://codereview.appspot.com/256070043/ >
Sign in to reply to this message.
Hi, sorry for the (very) late reply! The pull request is merged now. Thanks a lot and especially thanks for your patience! I can upload the changed version later to coderview.appspot.com. Is there anyone still maintaining the chromium branch? -- Andi Jason Robbins <jrobbins@google.com> schrieb am Do., 30. Juli 2015 um 18:33 Uhr: > I see that there is some activity on > https://groups.google.com/forum/#!forum/codereview-discuss > I think you were right to contact Andi. > > On Thu, Jul 30, 2015 at 9:17 AM, <twifkak@google.com> wrote: > >> On 2015/07/30 15:40:05, jrobbins (corp) wrote: >> >>> lgtm >>> >> >> This change looks like a good fix. I'm not going to commit it or >>> >> deploy it >> >>> because I am no longer active in the original rietveld project, so >>> >> you'll need >> >>> help from someone who is. >>> >> >> Oh, okay. Do you know who is? >> >> https://codereview.appspot.com/256070043/ >> > >
Sign in to reply to this message.
No problem, thanks for the any reply. :) I have no clue, but I'll ask rietveld-admins@ about it. On Thu, Jul 30, 2015 at 11:39 AM, Andi Albrecht <albrecht.andi@gmail.com> wrote: > Hi, sorry for the (very) late reply! > > The pull request is merged now. Thanks a lot and especially thanks for > your patience! > > I can upload the changed version later to coderview.appspot.com. > > Is there anyone still maintaining the chromium branch? > > -- > Andi > > Jason Robbins <jrobbins@google.com> schrieb am Do., 30. Juli 2015 um > 18:33 Uhr: > >> I see that there is some activity on >> https://groups.google.com/forum/#!forum/codereview-discuss >> I think you were right to contact Andi. >> >> On Thu, Jul 30, 2015 at 9:17 AM, <twifkak@google.com> wrote: >> >>> On 2015/07/30 15:40:05, jrobbins (corp) wrote: >>> >>>> lgtm >>>> >>> >>> This change looks like a good fix. I'm not going to commit it or >>>> >>> deploy it >>> >>>> because I am no longer active in the original rietveld project, so >>>> >>> you'll need >>> >>>> help from someone who is. >>>> >>> >>> Oh, okay. Do you know who is? >>> >>> https://codereview.appspot.com/256070043/ >>> >> >>
Sign in to reply to this message.
On Thu, Jul 30, 2015 at 11:39 AM, Andi Albrecht <albrecht.andi@gmail.com> wrote: > > Is there anyone still maintaining the chromium branch? > > I have not been maintaining the chromium branch in the official rietveld open source project repo on github. You could probably ignore that branch for the future. However, I and other chromium.org developers do maintain our fork that is used on codereview.chromium.org. https://chromium.googlesource.com/infra/infra/+/master/appengine/chromium_rie... Thanks, jason!
Sign in to reply to this message.
On Thu, Jul 30, 2015 at 12:19 PM, Jason Robbins <jrobbins@google.com> wrote: > However, I and other chromium.org developers do maintain our fork that is > used on codereview.chromium.org. > > https://chromium.googlesource.com/infra/infra/+/master/appengine/chromium_rie... > Ah, good to know. Will my commit appear there eventually, or do I need to do something to get it there?
Sign in to reply to this message.
(and deployed to codereview.chromium.org) On Thu, Jul 30, 2015 at 12:22 PM, Devin Mullins <twifkak@google.com> wrote: > On Thu, Jul 30, 2015 at 12:19 PM, Jason Robbins <jrobbins@google.com> > wrote: > >> However, I and other chromium.org developers do maintain our fork that >> is used on codereview.chromium.org. >> >> https://chromium.googlesource.com/infra/infra/+/master/appengine/chromium_rie... >> > > Ah, good to know. Will my commit appear there eventually, or do I need to > do something to get it there? >
Sign in to reply to this message.
On Thu, Jul 30, 2015 at 12:23 PM, Devin Mullins <twifkak@google.com> wrote: > (and deployed to codereview.chromium.org) > I just wrote a CL to make the same change in that fork. I usually deploy new versions to codereview.chromium.org weekly on Mondays. Thanks, jason!
Sign in to reply to this message.
The instance on codereview.appspot.com is now up-to-date. Thanks again for the fix! Jason Robbins <jrobbins@google.com> schrieb am Do., 30. Juli 2015 um 23:14 Uhr: > On Thu, Jul 30, 2015 at 12:23 PM, Devin Mullins <twifkak@google.com> > wrote: > >> (and deployed to codereview.chromium.org) >> > > I just wrote a CL to make the same change in that fork. > > I usually deploy new versions to codereview.chromium.org weekly on > Mondays. > ah, good to know! I was a bit puzzled what happened to the chromium branch/instance after the migration to github because it was so quiet. -- Andi > > Thanks, > jason! > >
Sign in to reply to this message.
|