Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1823753002:
SkPDF: record milestone version in Producer string (Closed)
|
Created:
4 years, 9 months ago by hal.canary Modified:
4 years, 8 months ago CC:
reviews_skia.org, bungeman-skia, mtklein Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionSkPDF: milestone in metadata, also SkMilestone.h
TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1823753002
Committed: https://skia.googlesource.com/skia/+/ffe54004b92f84b65ee4569aaccbc178c51b017f
Patch Set 1 #Patch Set 2 : 2016-03-28 (Monday) 16:13:43 EDT #Patch Set 3 : include #
Messages
Total messages: 31 (12 generated)
Description was changed from ========== SkPDF: record milestone version in Producer string ========== to ========== SkPDF: record milestone version in Producer string GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by halcanary@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1823753002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1823753002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
halcanary@google.com changed reviewers: + tomhudson@google.com
PTAL motivation: make debugging non-Chrome PDFs easier.
How are we going to maintain?
On 2016/03/22 12:21:23, tomhudson wrote: > How are we going to maintain? After each branch, I change it on the master branch.
tomhudson@google.com changed reviewers: + hcm@google.com
This overlaps heavily with https://codereview.chromium.org/1781673002/. Will Heather commit to making the milestone change part of the documented formal release process? I really don't want to land a milestone marker in the code that depends on you remembering to update it manually every six weeks, even if you are more conscientious than most of us. Apparently Infra doesn't have anything we could grab.
On 2016/03/22 13:10:47, tomhudson wrote: > This overlaps heavily with https://codereview.chromium.org/1781673002/. > > Will Heather commit to making the milestone change part of the documented formal > release process? I really don't want to land a milestone marker in the code that > depends on you remembering to update it manually every six weeks, even if you > are more conscientious than most of us. > > Apparently Infra doesn't have anything we could grab. Let's pause for a minute and think through what makes sense here. I don't mind the versioning but want something can be automated. There are also similar requests floating around to get some type of Skia version data in about:version and about:gpu.
On 2016/03/22 14:34:31, hcm wrote: > On 2016/03/22 13:10:47, tomhudson wrote: > > This overlaps heavily with https://codereview.chromium.org/1781673002/. > > > > Will Heather commit to making the milestone change part of the documented > formal > > release process? I really don't want to land a milestone marker in the code > that > > depends on you remembering to update it manually every six weeks, even if you > > are more conscientious than most of us. > > > > Apparently Infra doesn't have anything we could grab. > > Let's pause for a minute and think through what makes sense here. I don't mind > the versioning but want something can be automated. There are also similar > requests floating around to get some type of Skia version data in about:version > and about:gpu. What would an "automated" process look like? How would it know to update? I am having trouble envisioning how this would work.
hcm@, Is this close to what you were looking for?
Yes, the root is to have a constant that can be used in multiple places outside the PDF code. Since our clients want it, it might need to be in include/ rather than src/. I think the simple version would have been to add to Heather's checklist "Update SkMilestone.h". But if we want to go with excessive automation, this seems to me to be the right kind of scripting.
On 2016/03/28 20:23:19, tomhudson wrote: > Yes, the root is to have a constant that can be used in multiple places outside > the PDF code. > Since our clients want it, it might need to be in include/ rather than src/. > > I think the simple version would have been to add to Heather's checklist "Update > SkMilestone.h". > But if we want to go with excessive automation, this seems to me to be the right > kind of scripting. Like the globally accessible file & the automation, since people have a bit of a hard time following all the milestone branching steps. Agree with Tom's comment, can we move to include/ so that clients can access?
On 2016/03/29 13:42:59, hcm wrote: > [...] can we move to include/ so that clients can access? yes.
The CQ bit was checked by halcanary@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1823753002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1823753002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/03/29 14:20:05, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. lgtm
Feels like automation may be overkill, but LGTM.
halcanary@google.com changed reviewers: + reed@google.com
adding reed@ for include/
Description was changed from ========== SkPDF: record milestone version in Producer string GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== SkPDF: milestone in metadata, also SkMilestone.h GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== SkPDF: milestone in metadata, also SkMilestone.h GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== SkPDF: milestone in metadata, also SkMilestone.h TBR=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by halcanary@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1823753002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1823753002/40001
Message was sent while issue was closed.
Description was changed from ========== SkPDF: milestone in metadata, also SkMilestone.h TBR=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== SkPDF: milestone in metadata, also SkMilestone.h TBR=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/ffe54004b92f84b65ee4569aaccbc178c51b017f ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/ffe54004b92f84b65ee4569aaccbc178c51b017f |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
