Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
866133002:
Collect everything Skia into one document. (Closed)
|
Created:
5 years, 11 months ago by hal.canary Modified:
5 years, 11 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionCollect everything Skia into one document.
http://skiadocs.com:8000/dev/contrib/directory?cl=866133002
Committed: https://skia.googlesource.com/skia/+/68b60c3d5a0f45144ecaa21006e051dc75a9ec94
Patch Set 1 #Patch Set 2 : Another Patch Set #
Total comments: 19
Patch Set 3 : Another Patch Set #
Total comments: 1
Messages
Total messages: 14 (5 generated)
halcanary@google.com changed reviewers: + jcgregorio@google.com
good? bad?
mtklein@google.com changed reviewers: + mtklein@google.com
https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... File site/dev/contrib/skia-directory.md (right): https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:1: The Skia Directory Sort of seems redundant with the index on the left, sometimes with more information, sometimes less, sometimes with different names. I think I'd rather have the index, where clicking on, say [Skia Fiddle] takes you to a page that describes it and links to it, rather than just a link. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:5: - [Skia.org](http://skia.org/) One of these is a link to itself... https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:9: - [Skia Docs](http://skiadocs.com:8000/) ...and the other is deprecated.
https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... File site/dev/contrib/skia-directory.md (right): https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:1: The Skia Directory On 2015/01/22 21:22:20, mtklein wrote: > Sort of seems redundant with the index on the left, sometimes with more > information, sometimes less, sometimes with different names. I think I'd rather > have the index, where clicking on, say [Skia Fiddle] takes you to a page that > describes it and links to it, rather than just a link. I tend to agree. But until all of these links are easy to get to from skia.org (they're not), some documentation is better than no documentation. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:5: - [Skia.org](http://skia.org/) On 2015/01/22 21:22:20, mtklein wrote: > One of these is a link to itself... This document should stand on its own: you may be reading it in markdown form inside the repository. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:9: - [Skia Docs](http://skiadocs.com:8000/) On 2015/01/22 21:22:20, mtklein wrote: > ...and the other is deprecated. Right. I thought I needed this for the ?cl=... syntax, but I don't.
https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... File site/dev/contrib/skia-directory.md (right): https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:1: The Skia Directory On 2015/01/22 21:29:37, Hal Canary wrote: > On 2015/01/22 21:22:20, mtklein wrote: > > Sort of seems redundant with the index on the left, sometimes with more > > information, sometimes less, sometimes with different names. I think I'd > rather > > have the index, where clicking on, say [Skia Fiddle] takes you to a page that > > describes it and links to it, rather than just a link. > > I tend to agree. But until all of these links are easy to get > to from http://skia.org (they're not), some documentation is better > than no documentation. I agree with Hal, in that I'd rather have this info live first, we can decide later if we want to refactor it. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:3: Please rename this file to just directory.md. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:5: - [Skia.org](http://skia.org/) On 2015/01/22 21:29:37, Hal Canary wrote: > On 2015/01/22 21:22:20, mtklein wrote: > > One of these is a link to itself... > > This document should stand on its own: you may be reading it in markdown form > inside the repository. Link should be https://skia.org https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:6: - [Skia Issue Tracker](https://code.google.com/p/skia/issues/) Just [Issue Tracker] https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:8: Documentation](http://skia-autogen.googlecode.com/svn/docs/html/index.html) This link is a 404 for me. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:12: - [Skia git repository](https://skia.googlesource.com/skia/) Can we drop Skia from all the link descriptions below. This is served from the Skia site, so I think the read is going to know they are all about Skia. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:27: - [Skia Fiddle](https://skfiddle.com/) A very short sentence fragment after each of these to explain the sites function. I don't think an outside reader will know that Gold == correctness.
https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... File site/dev/contrib/skia-directory.md (right): https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:1: The Skia Directory On 2015/01/23 15:07:16, jcgregorio wrote: > On 2015/01/22 21:29:37, Hal Canary wrote: > > On 2015/01/22 21:22:20, mtklein wrote: > > > Sort of seems redundant with the index on the left, sometimes with more > > > information, sometimes less, sometimes with different names. I think I'd > > rather > > > have the index, where clicking on, say [Skia Fiddle] takes you to a page > that > > > describes it and links to it, rather than just a link. > > > > I tend to agree. But until all of these links are easy to get > > to from http://skia.org (they're not), some documentation is better > > than no documentation. > > I agree with Hal, in that I'd rather have this info live first, we can decide > later if we want to refactor it. sgtm
Patchset #4 (id:60001) has been deleted
Patchset #3 (id:40001) has been deleted
I say we commit and iterate. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... File site/dev/contrib/skia-directory.md (right): https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:3: On 2015/01/23 15:07:16, jcgregorio wrote: > Please rename this file to just directory.md. Done. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:5: - [Skia.org](http://skia.org/) On 2015/01/23 15:07:15, jcgregorio wrote: > On 2015/01/22 21:29:37, Hal Canary wrote: > > On 2015/01/22 21:22:20, mtklein wrote: > > > One of these is a link to itself... > > > > This document should stand on its own: you may be reading it in markdown form > > inside the repository. > > Link should be https://skia.org Done. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:6: - [Skia Issue Tracker](https://code.google.com/p/skia/issues/) On 2015/01/23 15:07:15, jcgregorio wrote: > Just [Issue Tracker] Done. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:8: Documentation](http://skia-autogen.googlecode.com/svn/docs/html/index.html) On 2015/01/23 15:07:16, jcgregorio wrote: > This link is a 404 for me. fixed. Not sure what happened. https://codereview.chromium.org/866133002/diff/20001/site/dev/contrib/skia-di... site/dev/contrib/skia-directory.md:12: - [Skia git repository](https://skia.googlesource.com/skia/) On 2015/01/23 15:07:15, jcgregorio wrote: > Can we drop Skia from all the link descriptions below. This is served from the > Skia site, so I think the read is going to know they are all about Skia. Done.
lgtm https://codereview.chromium.org/866133002/diff/80001/site/dev/contrib/directo... File site/dev/contrib/directory.md (right): https://codereview.chromium.org/866133002/diff/80001/site/dev/contrib/directo... site/dev/contrib/directory.md:7: - [Autogenerated API This might be just as clear as [Doxygen], but that's not a strong preference.
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/866133002/80001
Message was sent while issue was closed.
Committed patchset #3 (id:80001) as https://skia.googlesource.com/skia/+/68b60c3d5a0f45144ecaa21006e051dc75a9ec94 |
