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

Switch to reference style links #680

Merged
merged 6 commits into from
Sep 7, 2022

Conversation

wolf99
Copy link
Contributor

@wolf99 wolf99 commented Apr 28, 2022

Changing markdown URLs to markdown reference style links as suggested by @ldennington on #668 .

In some places, I have changed link text from strings like "here" as in download here to be more descriptive (and thus better URL reference IDs) like download at the .NET website.

Also on #668 I proposed that where aka.ms links are linking to other documents or files in this same repository, then they are replaced with relative links for the following benefits:

  1. Remove dependency on MS short link service
  2. Allow links to work offline.

@ldennington agreed with this, but requested a check with @mjcheetham (#668 (comment))

Can you confirm if you are happy with this, Mathew? If not, I will remove that from this PR.

@wolf99 wolf99 force-pushed the ref-style-links branch 3 times, most recently from 5e11b78 to 9dfa207 Compare May 2, 2022 11:20
Copy link
Collaborator

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @wolf99! Thanks for this. In principal I like the idea!

However I think that we should always try to give a "proper name" to the reference links, except where the word is self-descriptive (like libsecret) and is a simple ASCII A-Z string without punctuation*.

For example [.NET][] should be [.NET][dotnet] and [managed in the keychain][] should be [managed in the keychain][macos-keychain].

Keeps the prose less verbose, whilst also still making the reference link section at the bottom organised, easier to scan.

*except for the many many many credential.foo config links.. I really like the reference links here for cleaning these up!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
@wolf99
Copy link
Contributor Author

wolf99 commented May 4, 2022

Ha ha, I actually started out the first file or two with something very like this! 😄
I switched back because it seemed like cleaner prose and someone reading the URLs at the end of the file might have a bit more context.
But I'm happy to switch back to that also.

Copy link
Collaborator

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few more links/references that I'd like to see changed from prose to a short kebab-case ID and should be good to go! Thanks 😁

docs/credstores.md Outdated Show resolved Hide resolved
docs/enterprise-config.md Outdated Show resolved Hide resolved
docs/development.md Outdated Show resolved Hide resolved
docs/development.md Outdated Show resolved Hide resolved
docs/development.md Outdated Show resolved Hide resolved
docs/development.md Outdated Show resolved Hide resolved
@wolf99
Copy link
Contributor Author

wolf99 commented May 23, 2022

There's a good few files yet to get through converting to any reference style, I haven't got to those yet but planning to add to this PR as I get the time 🙂

@ldennington
Copy link
Contributor

Thanks for the heads up @wolf99! No rush at all.

@wolf99 wolf99 force-pushed the ref-style-links branch 2 times, most recently from 1697b10 to 9164efa Compare June 25, 2022 16:39
@wolf99
Copy link
Contributor Author

wolf99 commented Jun 25, 2022

Squashed commits because rebase is painful 😢

@wolf99 wolf99 force-pushed the ref-style-links branch 3 times, most recently from 6001804 to a128cae Compare August 1, 2022 21:24
@ldennington
Copy link
Contributor

Hey @wolf99! I see this version bump PR is failing with a few issues, some related to bare links. Wanted to reach out and see if you'd like some assistance finishing out the conversion to reference style links here?

@wolf99
Copy link
Contributor Author

wolf99 commented Aug 26, 2022

Hi @ldennington

I have not had much time to focus on FOSS things recently 😔.
I am still very interested to continue the work here and resolve any subsequent issues with the CI actions, when I get time.
That said, if anyone else would like to add commits on this PR to speed things up in the mean time, they would be very welcome to 👍.

@wolf99 wolf99 force-pushed the ref-style-links branch 2 times, most recently from fff626c to d905c6d Compare August 27, 2022 18:44
@wolf99
Copy link
Contributor Author

wolf99 commented Aug 27, 2022

Do you think this ToC could be removed?
No other docs file uses a ToC and the doc is not very long despite having many headings.

Preference to remove it is due to the large amount of duplication that would be involved in changing these heading links to reference style links.

https://github.com/wolf99/git-credential-manager/blob/f472dc7239f6360bbd025dd0d0d56de2078e1059/docs/hostprovider.md?plain=1#L23

The front matter is ~10% of the overall document.
So my personal opinion would be to remove the full front matter.

  1. The author and change dates are tracked by git - so lines 3-7 could be removed
  2. The changes and the contents of those changes are also covered by git history - so lines 9-14 could be removed
  3. The ToC could be removed per above - so lines 25-45 could be removed

@ldennington
Copy link
Contributor

Do you think this ToC could be removed? No other docs file uses a ToC and the doc is not very long despite having many headings. (The front matter is ~10% of the overall document)

Preference to remove it is due to the large amount of duplication that would be involved in changing these heading links to reference style links.

https://github.com/wolf99/git-credential-manager/blob/f472dc7239f6360bbd025dd0d0d56de2078e1059/docs/hostprovider.md?plain=1#L23

I'm fine with removing it!

@wolf99 wolf99 marked this pull request as ready for review August 27, 2022 20:19
@wolf99
Copy link
Contributor Author

wolf99 commented Aug 27, 2022

This is good for review!

Nope, there are still some prose ones hanging around that need to be converted to kebab case.

Now it's ready 😅

@wolf99 wolf99 marked this pull request as draft August 27, 2022 20:27
@wolf99 wolf99 marked this pull request as ready for review August 27, 2022 21:15
Copy link
Contributor

@ldennington ldennington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a TON of work - thank you so much for doing it! ⭐

I left a couple small comments, but looks great overall!

README.md Outdated Show resolved Hide resolved
docs/hostprovider.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
@wolf99
Copy link
Contributor Author

wolf99 commented Aug 30, 2022

Thanks for reviewing @ldennington.
Good comments, those changes are in the latest commit.

Copy link
Contributor

@ldennington ldennington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Looks awesome 🚀

@wolf99 wolf99 requested a review from mjcheetham August 30, 2022 20:51
Copy link
Collaborator

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thanks @wolf99 😁

@ldennington ldennington merged commit f230c46 into git-ecosystem:main Sep 7, 2022
@wolf99 wolf99 deleted the ref-style-links branch September 9, 2022 21:03
@ldennington ldennington mentioned this pull request Nov 3, 2022
ldennington added a commit that referenced this pull request Nov 3, 2022
Changes:

- Check for broken links in documentation (#700)
- Support macOS `arm64` installs via Homebrew (#798) 
- Validate installers before publishing (#813)
- Auto-generate maintainer away notification issues (#842)
- Install dotnet via Jammy feeds on Ubuntu 22.04 and greater (#839)
- Access Azure storage account using service principle credentials
(#851)
- Update documentation to use reference-style links (#680)
- Unify documentation line length (#862)
- Add generic username/password UI (#871)
- Bitbucket DC OAuth support (#607)
- Distribute GCM as a dotnet tool (#886)
- Drop `-core` suffix from entry executable #551 
- Speed up build graph (#924)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants