Skip to content

Conversation

@crazy-max
Copy link
Member

@crazy-max crazy-max commented May 22, 2022

It's quite hard to keep track of "edit url" in our configuration for remote resources. Some of them are not there, only match the parent folder or are broken like: https://docs.docker.com/desktop/extensions-sdk/dev/cli/usage/

image

Here the "Edit this page" button is relative to the current repository and not the upstream one: https://github.com/docker/docker.github.io/edit/master/desktop/extensions-sdk/dev/cli/usage.md

This PR makes some changes to our fetch remote resources plugin to automatically generate edit_url for the right scope when a resource is fetched remotely. This way we don't need to maintain them in our configuration.

In a follow-up we should also look at making this kind of automation for remote docs reference like https://docs.docker.com/engine/reference/commandline/buildx_bake/.

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

@netlify
Copy link

netlify bot commented May 22, 2022

Deploy Preview for docsdocker ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit c2ee94c
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/62a2fb21abf36c000acea7f3
😎 Deploy Preview https://deploy-preview-14799--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@crazy-max
Copy link
Member Author

@thaJeztah
Copy link
Member

only match the parent folder

I'd have to check the full content of this PR, but I know these can be a bit tricky;

  • Some of the external content (most notably the ones using the "yamldocs") may be based on a "snapshot" / specific version in time, so content (and files) upstream may have changed on the upstream "main" / "master" branch
  • However, we don't want pull requests against the upstream release-branches (instead have pull requests on "main" / "master", and backport if needed), otherwise the next release done from "main" / "master" would potentially miss the fix.
  • As most repositories (outside of the docs repo) require a DCO sign-off, I know I tried to avoid using "edit" links to the upstreams, as then the user would click "edit", and edit the file through GitHub's web UI, and open a PR without DCO sign-off. Linking to the "directory" still has that risk, but "tries" to at least make the contributor have a look at the repository before opening a PR

For that last one, perhaps the edit links could point to an intermediate page ("this page is maintained in repository X. Before opening a pull request, read the contribution guidelines for that repository" (with links to both?)) - just thinking out loud here. 🤔

@crazy-max
Copy link
Member Author

However, we don't want pull requests against the upstream release-branches (instead have pull requests on "main" / "master", and backport if needed), otherwise the next release done from "main" / "master" would potentially miss the fix.

Indeed it might not make sense. Maybe we could add an edit_url_ref in our fetch-remote block like:

fetch-remote:
  - repo: "https://github.com/docker/cli"
    ref: "20.10"
    edit_url_ref: "master"
    paths:

Or name it default_branch.

As most repositories (outside of the docs repo) require a DCO sign-off, I know I tried to avoid using "edit" links to the upstreams, as then the user would click "edit", and edit the file through GitHub's web UI, and open a PR without DCO sign-off. Linking to the "directory" still has that risk, but "tries" to at least make the contributor have a look at the repository before opening a PR

Yeah this one is tricky indeed. I guess that's why docs repo does not have DCO sign-off enabled? Maybe this is stupid but do you know if probot have a DCO config to exclude some paths for example?

@crazy-max
Copy link
Member Author

Yeah this one is tricky indeed. I guess that's why docs repo does not have DCO sign-off enabled? Maybe this is stupid but do you know if probot have a DCO config to exclude some paths for example?

Not sure but Third-party remediation support might do the trick: https://github.com/dcoapp/app#third-party-remediation-support

@crazy-max
Copy link
Member Author

crazy-max commented May 23, 2022

@usha-mandya
Copy link
Member

This is great. Thanks @crazy-max.

For Docker CLI docs, the Request docs changes link still points to the Docs repo. Should we also change this to point to the upstream CLI repo?

@crazy-max crazy-max force-pushed the auto-edit-url-remote branch 2 times, most recently from 6ae47c8 to cb745da Compare June 1, 2022 06:01
Comment on lines +107 to +111
- scope:
path: engine/reference
values:
# FIXME: This edit url is as "best-effort" and doesn't match anything in docker/cli repo for plugins for example. It should be generated autmatically.
edit_url: "https://github.com/docker/cli/tree/master/docs/reference"
Copy link
Member Author

@crazy-max crazy-max Jun 1, 2022

Choose a reason for hiding this comment

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

@thaJeztah As discussed, added back the "best-effort" edit url for engine references. In a follow-up we can see how to generate edit url for references from yaml files in _data folder.

@crazy-max crazy-max force-pushed the auto-edit-url-remote branch from cb745da to 075f276 Compare June 1, 2022 08:05
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max force-pushed the auto-edit-url-remote branch from 075f276 to c2ee94c Compare June 10, 2022 08:04
@crazy-max
Copy link
Member Author

As most repositories (outside of the docs repo) require a DCO sign-off, I know I tried to avoid using "edit" links to the upstreams, as then the user would click "edit", and edit the file through GitHub's web UI, and open a PR without DCO sign-off. Linking to the "directory" still has that risk, but "tries" to at least make the contributor have a look at the repository before opening a PR

Now that we can enforce DCO in the webui, I think we can move forward with this PR: https://github.blog/changelog/2022-06-08-admins-can-require-sign-off-on-web-based-commits/

@crazy-max
Copy link
Member Author

@thaJeztah Any blocker? This PR would be handy for #14948 so users would be redirected to the right page to open issues and edit the page from BuildKit repo.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

whoops, forgot we had this one 😅

@thaJeztah thaJeztah merged commit f540f58 into docker:master Jun 28, 2022
@crazy-max crazy-max deleted the auto-edit-url-remote branch June 28, 2022 10:26
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.

3 participants