Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrating to gcloud-common docs #734

Merged
merged 1 commit into from
Aug 5, 2015
Merged

Migrating to gcloud-common docs #734

merged 1 commit into from
Aug 5, 2015

Conversation

callmehiphop
Copy link
Contributor

Fixes #710
Fixes #717

For your viewing pleasure: http://callmehiphop.github.io/gcloud-node-ghpages/#/docs/master

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 21, 2015
@callmehiphop
Copy link
Contributor Author

@stephenplusplus Will we be deleting the authorization/faq markdown files within gcloud-node? It contains some information tailored to our library specifically, so I wasn't sure if wanted to have an additional auth link called something like "gcloud-node Authorization"

@jgeewax
Copy link
Contributor

jgeewax commented Jul 22, 2015 via email

@callmehiphop
Copy link
Contributor Author

It's just an angular directive, so we could include both on the same page and just trim the node specific one to remove any duplicated information.

@stephenplusplus
Copy link
Contributor

For the images, we need to change gcloud-comon to use the ![](path.png) style. For some reason, I used <img>. After that, we have a few options, ranked by how good of a solution I think they are (please correct me if I'm wrong!)

  1. Use absolute image paths in the gcloud-common markdown
  2. Use a showdown extension to replace img paths with absolute urls at runtime
  3. Use a directive to replace img paths with absolute urls at runtime

The extension would look like:

angular
  .module('gcloud.authorization', [/*...*/])
  .config(function($routeProvider, markdownConverterProvider) {
    'use strict';

    markdownConverterProvider.config({
      extensions: [function() {
        return [
          {
            type: 'output',
            regex: 'img src="',
            replace: 'img src="https://raw.githubusercontent.com/GoogleCloudPlatform/gcloud-common/master/authorization/'
          }
        ];
      }]
    });

  //...

The directive could look like:

angular
  .module('gcloud.detect-images', [])
  .directive('detectImages', function() {
    return function($scope, elem, attrs) {
      [].slice.call(elem.find('img')).forEach(function(img) {
        var src = img.getAttribute('src');
        img.setAttribute('src', attrs.detectImages + src);
      });
    };
  });
<btf-markdown
  highlight-markdown
  detect-images="https://raw.githubusercontent.com/GoogleCloudPlatform/gcloud-common/master/authorization/"
  ng-include="'https://raw.githubusercontent.com/GoogleCloudPlatform/gcloud-common/master/authorization/readme.md'">

@stephenplusplus
Copy link
Contributor

For the Node-specific docs, I think we will have to add them after the common docs.

For Authorization, the common docs end with downloading a JSON key file. We can add on right after that to show how you use it with our library, and the transition should be easy. Same for FAQ, we can just tack any custom questions after the common ones.

It's not awesome to have a dependency on the wording of the docs, so I suppose we will just have to keep our docs in mind anytime the common docs change.

@jgeewax
Copy link
Contributor

jgeewax commented Jul 22, 2015 via email

@callmehiphop
Copy link
Contributor Author

I ended up going with the directive option because:

  • The hardcoded absolute URL feels gross to me and it might end up only being useful to us and not the other gcloud-* projects
  • Configuring the provider doesn't promote re-use in case we need to do something similar for a different markdown file (e.g. we add images to faqs)

@callmehiphop
Copy link
Contributor Author

So I put the node specific auth stuff back in and it looks pretty good, however the faqs are kinda ugly now. Ideally I think we would split the Where can I get more help? section into its own markdown file because it would be better placed at the end of the page. There were also some small formatting differences (h2s became h4s) so it's a little harder to distinguish between questions and answers since the text looks the same.

@stephenplusplus any opposition to making some minor changes to the common faq?

@stephenplusplus
Copy link
Contributor

Change it up! We're the only ones implementing it so far (afaik), so let's
start with what works for us, and make changes as necessary as others join.
Also, we all share the same template, so it should be difficult to do
something for us that won't also benefit the others.

On Wednesday, July 22, 2015, Dave Gramlich notifications@github.com wrote:

So I put the node specific auth stuff back in and it looks pretty good,
however the faqs are kinda ugly now. Ideally I think we would split the Where
can I get more help?
section into its own markdown file because it would
be better placed at the end of the page. There were also some small
formatting differences (h2s became h4s) so it's a little harder to
distinguish between questions and answers since the text looks the same.

Any opposition to making some minor changes to the common faq?

// cc @stephenplusplus https://github.com/stephenplusplus


Reply to this email directly or view it on GitHub
#734 (comment)
.

@@ -2,7 +2,7 @@
<header header title="Authorization">
<div class="row row--right">
<div class="col margin-vertical">
<a href="https://github.com/GoogleCloudPlatform/gcloud-node/edit/master/docs/authorization.md"
<a href="https://github.com/GoogleCloudPlatform/gcloud-common/blob/master/authorization/readme.md"

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor

The hardcoded absolute URL feels gross to me and it might end up only being useful to us and not the other gcloud-* projects

I actually think it would help the other projects, since they should incorporate the live/master representation of gcloud-common in their docs instead of cloning locally. So, the image should always be remote, requiring an absolute path. Am I not considering something?

@callmehiphop
Copy link
Contributor Author

If another project chose to use submodules over absolute URLs, it would kinda defeat the point of using submodules since the markdown files would be referencing an absolute image path instead of the relative one.

@stephenplusplus
Copy link
Contributor

But I think if other projects used submodules, that would kind of defeat the point of the shared docs, since they would constantly have to be on the lookout for gcloud-common updates and manually do a submodule update and re-build of the site.

I know we have it easy, since we just point an Angular directive to a remote MD file, and boom, magic. But, for a site that doesn't have such an easy set-up, they will have to run some kind of MD -> HTML conversion as well, and the remote image will always work for everyone. Our docs are GitHub hosted and so are the -common images, so I think we should just be ¯_(ツ)_/¯ about it and use the absolute to dramatically simplify our site and wait to know it's a problem with others before making it harder on ourselves than it might need to be.

@stephenplusplus
Copy link
Contributor

It looks so awesome. How should we handle the links inside of the common MD? Specifically, these ones are problematic:

Contributing

@stephenplusplus
Copy link
Contributor

Is it possible that the common links change from:

/troubleshooting/readme.md

to

/troubleshooting (if browsing on GH, the readme will still be displayed)

Then we can have a base href of http://googlecloudplatform.github.io/gcloud-node/#/?

@callmehiphop
Copy link
Contributor Author

I think that would be a little bit tricky, <base> would mess with all of our dependency paths. Maybe for this one a Showdown plugin would be better?

@stephenplusplus
Copy link
Contributor

Oh yeah, that makes more sense.


'use strict';

Showdown.extensions.relativelink = function () {

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor

@callmehiphop I played with this locally using the new Showdown updates (using the rawgit cdn), and all seems well. Just a little quirk... I had to add this after including the file in index.html:

<script>
  Showdown = showdown
  Showdown.converter = showdown.Converter
 </script>

(btford/angular-markdown-directive#39)

@callmehiphop
Copy link
Contributor Author

@stephenplusplus yeah, that's because we're on a really old version of showdown, they changed the API a little in 1.x.x

@stephenplusplus
Copy link
Contributor

That's because of angular-markdown-directive. We can use this file from btford/angular-markdown-directive#39.

extensions: [function () {
return [{
type: 'lang',
// gah \[([^\]]+)\]\(([\/|\#][^)]+)\)

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor

Authorization >> Any ideas how we can make the subheadings non-tiny? (Re-use an existing...)

screen shot 2015-08-04 at 4 55 04 pm

@stephenplusplus
Copy link
Contributor

FAQ >> Looks like this list got busted. Is there also a way to indent and style those bullet points? (Google Cloud Datastore API, etc)

screen shot 2015-08-04 at 4 55 46 pm


if (isDeeplink) {
href = href.split(/[\#|\-]/).join('');
attribute = 'href="" ng-click="setSection(\'' + href + '\')"';

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor

Since you have figured out deeplinking (yay!!), can we add a clickable "#" such as here: http://callmehiphop.github.io/gcloud-node-ghpages/#/docs/master/bigquery/dataset next to createTable?

@stephenplusplus
Copy link
Contributor

I noticed when hitting http://callmehiphop.github.io/gcloud-node-ghpages/#/authorization?section=createanewserviceaccount in a new tab directly, it will scroll, but it won't be as accurate as if I clicked the link from within the doc.

@stephenplusplus
Copy link
Contributor

Related: #734 (comment)

CONTRIBUTING >> "Finding something to work on"

screen shot 2015-08-04 at 5 03 31 pm

@callmehiphop
Copy link
Contributor Author

Any ideas how we can make the subheadings non-tiny?

Currently they are <h6> tags, could we just bump them up to <h4>s

Looks like this list got busted. Is there also a way to indent and style those bullet points? (Google Cloud Datastore API, etc)

Looks like a showdown bug, we can increase the indent and it looks like that will work. (https://jsfiddle.net/xjeeLpmt/)

I noticed when hitting http://callmehiphop.github.io/gcloud-node-ghpages/#/authorization?section=createanewserviceaccount in a new tab directly, it will scroll, but it won't be as accurate as if I clicked the link from within the doc.

That looks like a race condition from the markdown not being fully rendered yet..

@callmehiphop
Copy link
Contributor Author

Since you have figured out deeplinking (yay!!), can we add a clickable "#" such as here: http://callmehiphop.github.io/gcloud-node-ghpages/#/docs/master/bigquery/dataset next to createTable?

@stephenplusplus We could, but I find it a little confusing since the doc permalinks hide all other content on the page.

created a showdown extension for converting relative links
localized showdown
created markdown directive
added deeplinking support
@stephenplusplus
Copy link
Contributor

We could, but I find it a little confusing since the doc permalinks hide all other content on the page.

Makes sense. I think it would actually be better if the docs didn't do that... think that's possible? (Can tackle that in another PR, if so)

@callmehiphop
Copy link
Contributor Author

Yeah I think so - let's do that in another PR, I'd like to put this one to bed soon :)

@stephenplusplus
Copy link
Contributor

Currently they are <h6> tags, could we just bump them up to <h4>s

That should be fine. I think I just made them 6s so they didn't overpower the numeric heading. Here they are as H4s:

screen shot 2015-08-04 at 9 35 06 pm

@callmehiphop
Copy link
Contributor Author

I ended up making them 3s, even 4s kinda looked a bit small. https://github.com/GoogleCloudPlatform/gcloud-common/pull/14

I refactored the deeplinking a bit and in the process I believe I addressed all of your comments. Let me know what you think!

stephenplusplus added a commit that referenced this pull request Aug 5, 2015
Migrating to gcloud-common docs
@stephenplusplus stephenplusplus merged commit 03512b2 into googleapis:master Aug 5, 2015
@stephenplusplus
Copy link
Contributor

It's gold. Props for taking on this massive PR and putting up with all of my comments along the way :)

@callmehiphop
Copy link
Contributor Author

woo, my pleasure 💃

@callmehiphop callmehiphop deleted the common-docs branch August 5, 2015 02:32
sofisl pushed a commit that referenced this pull request Jan 24, 2023
This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/a8c8456e-ad55-437d-a21c-2a0857049024/targets

- [ ] To automatically regenerate this PR, check this box. (May take up to 24 hours.)

Source-Link: googleapis/synthtool@e6f3d54
Source-Link: googleapis/synthtool@04573fd
Source-Link: googleapis/synthtool@c6706ee
Source-Link: googleapis/synthtool@b33b0e2
Source-Link: googleapis/synthtool@898b38a
sofisl pushed a commit that referenced this pull request Jan 25, 2023
This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/a8c8456e-ad55-437d-a21c-2a0857049024/targets

- [ ] To automatically regenerate this PR, check this box. (May take up to 24 hours.)

Source-Link: googleapis/synthtool@e6f3d54
Source-Link: googleapis/synthtool@04573fd
Source-Link: googleapis/synthtool@c6706ee
Source-Link: googleapis/synthtool@b33b0e2
Source-Link: googleapis/synthtool@898b38a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pull docs from gcloud-common Clarify API Enablement instructions in the Authorization section of readme.md
4 participants