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

Add tool to fix missing "source" links in our documentation #24389

Closed
wants to merge 2 commits into from

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jun 11, 2022

The problem (fixed in #24386) with broken links was that
the autoapi-generated source html were either exluded
(via example_dags/** exclusion) or not included (system_tests)
when the documentation was generated.

We cannot easily re-add and regenerated all those autoapi sources
so instead the fix is to replace all the links to autoapi sources
with corresponding links to raw github sources. We can easily do
that using tags of providers and link to specific tagged versions
of those example dags and this is the best we can do now.

The #24386 brings back linking to autoapi-generated html pages.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragement file, named {pr_number}.significant.rst, in newsfragments.

@potiuk potiuk requested review from eladkal, josh-fell and kaxil June 11, 2022 12:54
potiuk added a commit to apache/airflow-site that referenced this pull request Jun 11, 2022
potiuk added a commit to apache/airflow-site that referenced this pull request Jun 11, 2022
@potiuk potiuk force-pushed the add-tools-for-fixing-source-links branch from ed8ea08 to eb2b89e Compare June 11, 2022 13:00
@potiuk potiuk requested a review from mik-laj June 11, 2022 13:01
potiuk added a commit to apache/airflow-site that referenced this pull request Jun 12, 2022
@potiuk potiuk force-pushed the add-tools-for-fixing-source-links branch from eb2b89e to 68d7182 Compare June 12, 2022 19:55
@ashb
Copy link
Member

ashb commented Jun 13, 2022

Do we need this in the repo? Once we've run it we don't need it anymore do we?

@potiuk
Copy link
Member Author

potiuk commented Jun 13, 2022

Do we need this in the repo? Once we've run it we don't need it anymore do we?

It's good for reference to find it. Also @josh-fell used them later as a reference and good "source" of what has been fixed in building astronomer index. I find it useful to keep it there, in case someone wants to have similar fix in the future. First time it took me quite some time to figure out how to do, second time it was couple of minutes precisely because I could find it easily how it was done before.

@potiuk
Copy link
Member Author

potiuk commented Jun 13, 2022

Also those are useful in case somoen finds a problem with such mass-updated changes like you did apache/airflow-site#613 (comment) @ashb - (I will look into that).

If we've already merged the change and if there is a mistake that would lead to wrong corrections of the link, the easiest way to find why and fix it is to have reference to the script that was used. Otherwise any diagnosis and fix by anyone else but me hoping to still keep the script around would be much more difficult.

So say this is a way to not loose time with any followups.

potiuk added a commit to apache/airflow-site that referenced this pull request Jun 13, 2022
@potiuk potiuk force-pushed the add-tools-for-fixing-source-links branch from 68d7182 to 9462dfb Compare June 13, 2022 21:38
@potiuk
Copy link
Member Author

potiuk commented Jun 13, 2022

The tool is optimized now

potiuk added a commit to apache/airflow-site that referenced this pull request Jun 14, 2022
@potiuk
Copy link
Member Author

potiuk commented Jun 14, 2022

Applied the latest fix @ashb .

I'd very much like to keep those scripts in, actually (as explained - there might be errors we missed during the review, the apache/airflow-site#613 has reference to this PR and scripts used to fix the problems so we have an easy way to revert and re-run the fixes in case we find out that we need to. The interlinking in PRs make it permanent trace to all such changes for anyone who might want to wonder where it came from (and might even fix it if I am not around).

@potiuk
Copy link
Member Author

potiuk commented Jun 21, 2022

@ashb ?

potiuk added a commit to apache/airflow-site that referenced this pull request Jun 24, 2022
@potiuk
Copy link
Member Author

potiuk commented Jul 12, 2022

I would love to get it in (just for tracking of what has been done).

@potiuk
Copy link
Member Author

potiuk commented Jul 22, 2022

Would love to merge this one - we have one more case that might need fixes and it would be great to get this one as a base.

potiuk added 2 commits August 4, 2022 18:21
The problem (fixed in apache#24386) with broken links was that
the autoapi-generated source html were either exluded
(via example_dags/** exclusion) or not included (system_tests)
when the documentation was generated.

We cannot easily re-add and regenerated all those autoapi sources
so instead the fix is to replace all the links to autoapi sources
with corresponding links to raw github sources. We can easily do
that using tags of providers and link to specific tagged versions
of those example dags and this is the best we can do now.

The apache#24386 brings back linking to autoapi-generated html pages.
@potiuk potiuk force-pushed the add-tools-for-fixing-source-links branch from 7858a23 to 9607123 Compare August 4, 2022 16:21
@potiuk potiuk closed this Aug 4, 2022
@potiuk potiuk deleted the add-tools-for-fixing-source-links branch August 8, 2022 08:03
potiuk added a commit to apache/airflow-site that referenced this pull request Jun 17, 2023
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.

2 participants