Rename extension directories main .md files to README.md #1648

britice · 2016-01-28T21:50:56Z

Addresses #1304.

britice · 2016-01-28T21:56:33Z

I hacked together a bash script for this.

#!/usr/bin/env bash

echo "changing to "$1
cd $1

for i in $(find . -maxdepth 2 -name "*.md")
do
  b=$(basename $i)
  base=${b%.*}
  dir=$(dirname $i)
  parent_dir_name_regex="[\/]+"$base"$"
  if [[ $dir =~ $parent_dir_name_regex ]]; then
    echo "git mv"
    echo $i
    echo "$dir/README.md"
    git mv $i $dir/README.md
  fi
done

I manually checked the renaming but if you notice something wrong with the script, that may translate to incorrect renaming.

jridgewell · 2016-01-28T22:22:23Z

Doing a grep search (grep -aR 'extensions/[a-z-]\+/[a-z-]\+.md' *), looks like the following files have links that will need to be updated:

docs/include_features.md
README.md
spec/amp-html-components.md
spec/amp-iframe-origin-policy.md
spec/amp-var-substitutions.md
validator/testdata/feature_tests/amp_brightcove.html
validator/testdata/feature_tests/amp_brightcove.out
validator/testdata/feature_tests/amp_facebook.html
validator/testdata/feature_tests/amp_font.html
validator/testdata/feature_tests/amp_list.out
validator/testdata/feature_tests/amp_pinterest.html
validator/testdata/feature_tests/amp_user_notification.out
validator/testdata/feature_tests/mandatory_dimensions.out
validator/testdata/feature_tests/no_custom_js.out
validator/testdata/feature_tests/noscript.out
validator/testdata/feature_tests/regexps.out
validator/testdata/feature_tests/template.out
validator/testdata/feature_tests/youtube.html
validator/testdata/feature_tests/youtube.out
validator/validator.protoascii

Looks like you'll also need to rebase off of the the upstream master.

britice · 2016-01-29T01:07:08Z

@jridgewell, I made the replacements and rebased onto master. I used a slightly different regex (extensions\/([a-z-]+)\/\1\.md) so that amp-access-spec.md would not be included in the change. If we want references to that file to point to what was amp-access.md, then I'll need to replace those occurrences as well.

jridgewell · 2016-01-29T01:24:03Z

I used a slightly different regex (extensions/([a-z-]+)/\1.md)

Fancy. 😉

Still seeing one more in extensions/amp-analytics/README.md pointing to amp-user-notification. Also, (I should have mentioned before) we require a single commit to merge. Mind squashing?

cramforce · 2016-01-29T01:55:16Z

Scary PR. Thanks for doing this!

Are @Meggin @pbakaus and @erwinmombay on board with this for the docs-site?

britice · 2016-01-29T15:26:23Z

Still seeing one more in extensions/amp-analytics/README.md pointing to amp-user-notification.

Ah, nice catch. That must have slipped in after the rebase and I didn't rerun the search.

britice · 2016-01-29T15:37:33Z

Replacement made, rebased on master, and squashed.

@jridgewell, ready for another check. Hopefully I got everything this time. 😄

britice · 2016-01-29T16:11:31Z

Looks like the continuous-integration check is failing. The failure is coming from The command "gulp validator" exited with 1. A lot of the renaming took place in the validator directory but I'm still familiarizing myself with the code. Any suggestions for where to start with fixing things to pass that check?

Meggin · 2016-01-29T17:35:35Z

@pbakaus and @erwinmombay are the best people to check this error.

Important realization this morning-- not only will the build script need updating on the /docs side of things, but all the cross-references to the files throughout the docs will be broken, and this isn't good. We need to coordinate the changes to the filenames and the /docs script and the links at the same time so that the site isn't super-broken. Just to re-iterate, if we merge just the README changes, the current links in the site will be broken. And that's not good.

Meggin · 2016-01-29T17:37:53Z

Brian-- once we hear back from Erwin or Paul on how to fix the gulp error, I can sync with you to make sure we get the /docs links ready for README changes. We just need to coordinate together.

britice · 2016-01-29T18:41:14Z

... not only will the build script need updating on the /docs side of things, but all the cross-references to the files throughout the docs will be broken, and this isn't good.

Meggin-- This may be handled with how I modified the import_reference_docs.js script in the docs repo. Here is the compare of the branch which contains those changes.

I have it set up to take in the README.md for each extension and then convert the name to the name of the extension. For example, /extensions/amp-font/README.md is saved to _reference/extended/amp-font.md. This would keep any existing references to _reference/extended/amp-font.md functioning as expected in the site.

However, if there are any full references in the docs site to an extension .md file in the github repo (e.g. https://github.com/ampproject/amphtml/blob/master/extensions/amp-font/amp-font.md), I guess those would be broken.

britice · 2016-01-29T18:44:12Z

For convenience, here is the issue for handling the renaming in the docs repo: ampproject/amp.dev#20.

Meggin · 2016-01-29T18:47:44Z

Nice, Brian. Once we here back about gulp error, let's coordinate our
releases.

Basically once the READMEs go live, I need to run the script right away,
and create an internal cl that gets approved right away to update
ampproject.org.

The longer this doesn't happen, the longer we've got broken links.

Meggin

On Fri, Jan 29, 2016 at 10:44 AM, Brian Tice notifications@github.com
wrote:

For convenience, here is the issue for handling the renaming in the docs
repo: ampproject/amp.dev#20 ampproject/amp.dev#20.

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

britice · 2016-01-29T19:00:42Z

Meggin-- Sounds good. I'm not familiar with the release strategies for amp but I'll do my best to make sure you're in the loop before this gets merged into master. I guess @jridgewell just needs to be aware of the coordination requirement.

erwinmombay · 2016-01-29T19:37:00Z

the validator error is caused by https://github.com/ampproject/amphtml/blob/master/validator/validator_test.js#L142 we'll probably need to fix this internally. i can make the changes.

/cc @powdercloud

jridgewell · 2016-01-29T19:38:04Z

This LGTM, deferring to @erwinmombay to merge when the docs site can be updated.

britice · 2016-01-29T19:49:03Z

For convenience, here is the PR for changes on the docs side: ampproject/amp.dev#26.

erwinmombay · 2016-01-29T23:33:23Z

hey all, so i'm wondering if we should leave the extension/extension-name/extension-name.md files for now so we don't have any invalid links at any time; and once we have updated the validator in production we can remove the extension/extension-name/extension-name.md and only the README.md files will exist.

The other option is of course wait until the validator goes live and then merge this. (somebody will have to pay attention to when the server code goes live and then merge this)

let me know.

/cc @Gregable @powdercloud

Meggin · 2016-01-29T23:37:36Z

+1 to leaving files until both PRs are merged. Simple way to keep stuff
from being broken.

On Fri, Jan 29, 2016 at 3:33 PM, erwin mombay notifications@github.com
wrote:

hey all, so i'm wondering if we should leave the extension/extension-name/
extension-name.md files for now so we don't have any invalid links at any
time; and once we have updated the validator in production we can remove
the extension/extension-name/extension-name.md and only the README.md
files will exists.

The other option is of course wait until the validator goes live and then
merge this. (somebody will have to pay attention to when the server code
goes live and then merge this)

let me know.

/cc @Gregable https://github.com/Gregable @powdercloud
https://github.com/powdercloud

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

powdercloud · 2016-01-30T01:39:04Z

For the validator: I think we need to point the links to ampproject.org anyways. I'd rather do that than some refactoring still pointing at the github pages. I hope that the ampproject.org links will be stable and that pages that move will be configured with 301s, etc. Perhaps at some point some of these reference docs can be translated. Best of all: they're AMP Pages!!! :-)
So perhaps we could say that we should take care of repointing the validator errors to ampproject.org first, and then this pr no longer needs to touch it?

powdercloud · 2016-01-30T01:52:00Z

I filed #1687 to document the intent to fixup the validator spec links.

erwinmombay · 2016-02-03T18:59:52Z

@Meggin are you ok with the proposed change?

pbakaus · 2016-03-23T00:45:48Z

Can somebody resolve the conflicts and give me a quick overview of what this PR is trying to achieve? I've recently made some fixes to the importer that are of similar nature (I wrote the original importer).

powdercloud · 2017-01-12T18:54:35Z

I think this one is obsolete, so I'm closing it. Please feel free to reopen or comment if you disagree. Thanks much!

dvoytenko assigned JustinPinner and jridgewell and unassigned JustinPinner Jan 28, 2016

dvoytenko added the NEEDS REVIEW label Jan 28, 2016

britice force-pushed the readme-renames branch from ac00c50 to 08715cf Compare January 29, 2016 01:01

Rename extension directories main .md files to README.md

e681603

britice force-pushed the readme-renames branch from 08715cf to e681603 Compare January 29, 2016 15:32

jridgewell assigned erwinmombay and unassigned jridgewell Jan 29, 2016

erwinmombay removed the NEEDS REVIEW label Dec 7, 2016

powdercloud closed this Jan 12, 2017

elijahsoria mentioned this pull request May 5, 2021

🏗 SwG Release 5/5/21 #34238

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename extension directories main .md files to README.md #1648

Rename extension directories main .md files to README.md #1648

britice commented Jan 28, 2016

britice commented Jan 28, 2016

jridgewell commented Jan 28, 2016

britice commented Jan 29, 2016

jridgewell commented Jan 29, 2016

cramforce commented Jan 29, 2016

britice commented Jan 29, 2016

britice commented Jan 29, 2016

britice commented Jan 29, 2016

Meggin commented Jan 29, 2016

Meggin commented Jan 29, 2016

britice commented Jan 29, 2016

britice commented Jan 29, 2016

Meggin commented Jan 29, 2016

britice commented Jan 29, 2016

erwinmombay commented Jan 29, 2016

jridgewell commented Jan 29, 2016

britice commented Jan 29, 2016

erwinmombay commented Jan 29, 2016

Meggin commented Jan 29, 2016

powdercloud commented Jan 30, 2016

powdercloud commented Jan 30, 2016

erwinmombay commented Feb 3, 2016

pbakaus commented Mar 23, 2016

powdercloud commented Jan 12, 2017

Rename extension directories main .md files to README.md #1648

Rename extension directories main .md files to README.md #1648

Conversation

britice commented Jan 28, 2016

britice commented Jan 28, 2016

jridgewell commented Jan 28, 2016

britice commented Jan 29, 2016

jridgewell commented Jan 29, 2016

cramforce commented Jan 29, 2016

britice commented Jan 29, 2016

britice commented Jan 29, 2016

britice commented Jan 29, 2016

Meggin commented Jan 29, 2016

Meggin commented Jan 29, 2016

britice commented Jan 29, 2016

britice commented Jan 29, 2016

Meggin commented Jan 29, 2016

britice commented Jan 29, 2016

erwinmombay commented Jan 29, 2016

jridgewell commented Jan 29, 2016

britice commented Jan 29, 2016

erwinmombay commented Jan 29, 2016

Meggin commented Jan 29, 2016

powdercloud commented Jan 30, 2016

powdercloud commented Jan 30, 2016

erwinmombay commented Feb 3, 2016

pbakaus commented Mar 23, 2016

powdercloud commented Jan 12, 2017