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

Replace EDoc-issued <tt> elements with <code> (in generated HTML) #7643

Merged

Conversation

paulo-ferraz-oliveira
Copy link
Contributor

@paulo-ferraz-oliveira paulo-ferraz-oliveira commented Sep 10, 2023

Motivation

Erlang EDoc's External links mentions <a href="http://www.w3c.org/"><tt>http://www.w3c.org/</tt></a>.

Via jelly-beam/rebar3_ex_doc#66 I found <tt> is "deprecated" ("some browsers might still support it, it may have already been removed from the relevant web standards") and this has even been addressed in at least one commit in this repository.

This pull request attempts at completely moving away from <tt> in the scope of EDoc, to use semantic equivalents, all <code>. (I specifically mention this scope, because a few tt will still remain in other parts of the doc - if you want I can also pull request to remove those).

Other considerations

During this, I understood @type is deprecated in favor of -type. Should I make a new pull request to get rid of mentions to @type from the documentation? The code can also be scheduled for hard deprecation (I don't know when that warning was introduced) or simply removed (if it's there for more than 3 versions) and documented as a potential backward incompatibility.

I separated this into several commits because I wanted to test changes as I went but I can easily squash them if need be.

Edit: I'm not sure this should target maint(?) but it seems we can live a while longer with something that: a. is working still, b. has already been deprecated for a while.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 10, 2023

CT Test Results

    2 files    13 suites   4m 41s ⏱️
145 tests 139 ✔️ 6 💤 0
161 runs  155 ✔️ 6 💤 0

Results for commit 901a320.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@rickard-green rickard-green added the team:VM Assigned to OTP team VM label Sep 11, 2023
@garazdawi garazdawi added testing currently being tested, tag is used by OTP internal CI bug Issue is reported as a bug labels Sep 27, 2023
@garazdawi
Copy link
Contributor

garazdawi commented Sep 27, 2023

Thanks for this PR. I think we can make it target maint so that the docs and generation is updated as soon as possible.

Should I make a new pull request to get rid of mentions to @type from the documentation?

@type was deprecated in OTP 24, but there are still a lot of projects that use it, so I think we can leave it for a while yet. With the ongoing work to bring EEP-59 to Erlang a lot of things will change in this area so once that is merged we can again start looking at trimming/updating edoc.

@paulo-ferraz-oliveira
Copy link
Contributor Author

paulo-ferraz-oliveira commented Sep 27, 2023

Thanks for looking at this, @garazdawi.

@type was deprecated in OTP 24, but there are still a lot of projects that use it

Sure. My goal (initially) wasn't to remove it from OTP 😄 (even though I mention it as a possibility), but the documentation alone. Given your comment, maybe we could at least mark it as deprecated in the documentation (?) That'd be for another PR, though.

Edit: I've rebased on top of maint.

What about "tt will still remain in other parts of the doc - if you want I can also pull request to remove those" (?)

This is context-dependant, so I used code everywhere which is
what seem to make th most sense
e.g. [https://erlang.org]

edoc is, funny enough, code backwards :-)
e.g. %% @param Var The variable
% @throws 'Something'
e.g. %% @type myList(X). A special kind of lists ...
When no EDoc is actually present

e.g. id(Var) -> Var.
e.g. % @type str() = string().
e.g. % @equiv sth(Var, [])
(just define a behavior module to see how it works in terms of output)
e.g. -behaviour(gen_server).
e.g. %% @author carlsson.richard@gmail.com [http://example.net/richardc/]
@garazdawi
Copy link
Contributor

What about "tt will still remain in other parts of the doc - if you want I can also pull request to remove those" (?)

sure, please do that.

@paulo-ferraz-oliveira
Copy link
Contributor Author

I'll do that in a subsequent pull request, then, to not have scope creep 😄

@garazdawi garazdawi added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Sep 29, 2023
@garazdawi garazdawi merged commit 9f7efe7 into erlang:maint Oct 3, 2023
15 checks passed
@garazdawi
Copy link
Contributor

Thanks!

@garazdawi garazdawi added this to the OTP-26.2 milestone Oct 3, 2023
@paulo-ferraz-oliveira
Copy link
Contributor Author

Thank you, Lukas. I'll get on the other one 👍

@paulo-ferraz-oliveira paulo-ferraz-oliveira deleted the feature/edoc-code-for-tt branch October 3, 2023 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants