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

MINOR: Fix broken links in contrib guide #3135

Merged
merged 2 commits into from Aug 16, 2022

Conversation

andygrove
Copy link
Member

@andygrove andygrove commented Aug 13, 2022

Which issue does this PR close?

N/A

Rationale for this change

Fix broken links in published docs.

Before

- [dev/rust_lint.sh](../../../dev/rust_lint.sh)

After

- [dev/rust_lint.sh](https://github.com/apache/arrow-datafusion/blob/master/dev/rust_lint.sh)

What changes are included in this PR?

  • Add a new script to generate HTML which will rewrite the relative URLs in the contributor guide as absolute so that they work when published to the arrow web site.

This script only edits one file because there is only file with such relative links:

$ find temp -name "*.md" -exec grep -Hi "\.\.\/" {} \;
temp/contributor-guide/index.md:- [ci/scripts/rust_fmt.sh](../../../ci/scripts/rust_fmt.sh)
temp/contributor-guide/index.md:- [ci/scripts/rust_clippy.sh](../../../ci/scripts/rust_clippy.sh)
temp/contributor-guide/index.md:- [ci/scripts/rust_toml_fmt.sh](../../../ci/scripts/rust_toml_fmt.sh)
temp/contributor-guide/index.md:- [dev/rust_lint.sh](../../../dev/rust_lint.sh)
temp/contributor-guide/index.md:These randomly generate a parquet file, and then benchmark queries sourced from [parquet_query_sql.sql](../../../datafusion/core/benches/parquet_query_sql.sql) against it. This can therefore be a quick way to add coverage of particular query and/or data paths.
temp/contributor-guide/index.md:Instructions and tooling for running upstream benchmark suites against DataFusion can be found in [benchmarks](../../../benchmarks).
temp/contributor-guide/index.md:  - [here](../../../datafusion/physical-expr/src/string_expressions.rs) for string functions
temp/contributor-guide/index.md:  - [here](../../../datafusion/physical-expr/src/math_expressions.rs) for math functions
temp/contributor-guide/index.md:  - [here](../../../datafusion/physical-expr/src/datetime_expressions.rs) for datetime functions
temp/contributor-guide/index.md:  - create a new module [here](../../../datafusion/physical-expr/src) for other functions
temp/contributor-guide/index.md:- In [core/tests/sql](../../../datafusion/core/tests/sql), add a new test where the function is called through SQL against well known data and returns the expected result.
temp/contributor-guide/index.md:- In [expr/src/expr_fn.rs](../../../datafusion/expr/src/expr_fn.rs), add:
temp/contributor-guide/index.md:- In [core/src/logical_plan/mod](../../../datafusion/core/src/logical_plan/mod.rs), add:
temp/contributor-guide/index.md:  - [here](../../../datafusion/physical-expr/src/string_expressions.rs) for string functions
temp/contributor-guide/index.md:  - [here](../../../datafusion/physical-expr/src/math_expressions.rs) for math functions
temp/contributor-guide/index.md:  - [here](../../../datafusion/physical-expr/src/datetime_expressions.rs) for datetime functions
temp/contributor-guide/index.md:  - create a new module [here](../../../datafusion/physical-expr/src) for other functions
temp/contributor-guide/index.md:- In [datafusion/expr/src](../../../datafusion/expr/src/aggregate_function.rs), add:
temp/contributor-guide/index.md:- In [tests/sql](../../../datafusion/core/tests/sql), add a new test where the function is called through SQL against well known data and returns the expected result.

Are there any user-facing changes?

@andygrove
Copy link
Member Author

@kmitchener fyi

@andygrove andygrove added the documentation Improvements or additions to documentation label Aug 13, 2022
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Aug 13, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2022

Codecov Report

Merging #3135 (fdc3430) into master (e57dead) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3135   +/-   ##
=======================================
  Coverage   85.88%   85.88%           
=======================================
  Files         291      291           
  Lines       52770    52770           
=======================================
  Hits        45321    45321           
  Misses       7449     7449           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@andygrove andygrove added the documentation Improvements or additions to documentation label Aug 13, 2022
@andygrove andygrove changed the title MINOR: Fix broken links in contrb guide MINOR: Fix broken links in contrib guide Aug 13, 2022
- [ci/scripts/rust_fmt.sh](../../../ci/scripts/rust_fmt.sh)
- [ci/scripts/rust_clippy.sh](../../../ci/scripts/rust_clippy.sh)
- [ci/scripts/rust_toml_fmt.sh](../../../ci/scripts/rust_toml_fmt.sh)
- [ci/scripts/rust_fmt.sh](https://github.com/apache/arrow-datafusion/blob/master/ci/scripts/rust_fmt.sh)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change does mean that they won't work locally on people's machines, but not sure how much of an issue that is... It would be nice if we could have both work

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I could add a script to copy the files, search and replace the links, then build the HTML ...

Copy link
Member Author

Choose a reason for hiding this comment

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

@tustvold I changed approach here and added a script to fix up the links when publishing

@andygrove andygrove marked this pull request as draft August 14, 2022 19:53
@andygrove andygrove mentioned this pull request Aug 15, 2022
17 tasks
@andygrove andygrove marked this pull request as ready for review August 15, 2022 22:22
@andygrove
Copy link
Member Author

@alamb I'd like to get this in before releasing 11.0.0

@alamb alamb merged commit ccd9406 into apache:master Aug 16, 2022
@ursabot
Copy link

ursabot commented Aug 16, 2022

Benchmark runs are scheduled for baseline = e57dead and contender = ccd9406. ccd9406 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@andygrove andygrove deleted the fix-links-contrib-guide branch January 27, 2023 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants