Skip to content

Link to impersonation cookbook article #719

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

Merged
merged 1 commit into from
Apr 28, 2025
Merged

Conversation

ravage84
Copy link
Member

I war reading DerEuroMark's recent article about his TinyAuth plugin, which mentions the impersonation feature built into the Authorization plugin and wanted to look up the API. After that, I wanted to look up the documentation for that feature.

Having the links in the API to the docs helps devs to find what they look for.

@ravage84 ravage84 added this to the 3.next milestone Apr 28, 2025
@ravage84 ravage84 self-assigned this Apr 28, 2025
@jamisonbryant
Copy link
Contributor

@see tags are typically reserved for linking related classes. I believe the @link tag would be more appropriate.

Also worth nothing that we rarely link anywhere except the main Cake website, the PHP docs, or a few IETF RFCs. I feel like linking between code and docs creates a potential fragility when either the docs get updated, or more methods get added that reference those docs. It will be a chore to keep up-to-date. What if URLs change? Also, we're specifically endorsing 'en' language by using these links, are we not?

@dereuromark
Copy link
Member

dereuromark commented Apr 28, 2025

The en part is fine imo. I would usually prefer latest/ for latest docs
That often solves the wrong urls.
But can also be a follow up topic.

@jamisonbryant
Copy link
Contributor

The en part is fine imo

If others are fine with 'en', I am too, I just wanted to point it out since we're a multi-lingual framework.

I would usually prefer latest/ for latest docs

It seems those are not live yet, as I tried a few different URLs and they all 404'd.

@ravage84
Copy link
Member Author

@see tags are typically reserved for linking related classes. I believe the @link tag would be more appropriate.

As far as the current guide of phpDoc goes, both, @seeand @link, can be used for that:

Also worth nothing that we rarely link anywhere except the main Cake website, the PHP docs, or a few IETF RFCs. I feel like linking between code and docs creates a potential fragility when either the docs get updated, or more methods get added that reference those docs. It will be a chore to keep up-to-date. What if URLs change? Also, we're specifically endorsing 'en' language by using these links, are we not?

I personally have added a few links like this in the framework code to the docs over the past 12 years or so.
Always in English because it's the most updated one and to the code-compatible version of the docs respectively.

A quick search for both annotations linking to the docs for the framework code:

But yeah, it's an small, added chore when things change, which they don't do that often.

Also, we could harmonize the use of @link or @see for that.

@LordSimal
Copy link
Contributor

LordSimal commented Apr 28, 2025

It seems those are not live yet, as I tried a few different URLs and they all 404'd.

I have the branch ready for the NGINX config to enable that feature, just need write permissions on the site-book repo to push the branch.

@LordSimal
Copy link
Contributor

/latest URLs work now for the core book and all plugins books. See cakephp/site-book#1

@ravage84
Copy link
Member Author

/latest URLs work now for the core book and all plugins books. See cakephp/site-book#1

Thanks!

Though, someone working with an older verion of the code, reading through the API, would be linked to the too new documentation.
Thus, I would prefer to keep it as proposed.

@markstory markstory merged commit 6e4d461 into 3.x Apr 28, 2025
8 checks passed
@markstory markstory deleted the impersionation-cookbook-links branch April 28, 2025 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants