Skip to content

Conversation

@amoeba
Copy link
Member

@amoeba amoeba commented Feb 8, 2024

Rationale for this change

I came across this use of the word "cutely" and thought it might trip people up. I think the author was trying to make a point about CPU cache-friendliness and I don't think cutely is a common enough way to talk about CPU caches to be used here.

What changes are included in this PR?

It might be more specific to say that it's the data that's in buffers of the chunks that is either in the CPU cache or not but I think the simpler language I went with matches the high-level nature of this document.

Are these changes tested?

No

Are there any user-facing changes?

Yes, just to docs.

@kou
Copy link
Member

kou commented Feb 9, 2024

@ksuarez1423 Could you review this as the author of the original text?

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

+1, thank you

@pitrou pitrou merged commit 135b364 into apache:main Feb 12, 2024
@pitrou pitrou removed the awaiting review Awaiting review label Feb 12, 2024
@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Feb 12, 2024
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 135b364.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
### Rationale for this change

I came across this use of the word "cutely" and thought it might trip people up. I think the author was trying to make a point about CPU cache-friendliness and I don't think cutely is a common enough way to talk about CPU caches to be used here.

### What changes are included in this PR?

It might be more specific to say that it's the data that's in buffers of the chunks that is either in the CPU cache or not but I think the simpler language I went with matches the high-level nature of this document.

### Are these changes tested?

No

### Are there any user-facing changes?

Yes, just to docs.

Authored-by: Bryce Mecum <petridish@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
amoeba added a commit that referenced this pull request May 1, 2024
### What changes are included in this PR?

This adds developer tooling to the repo for linting the docs by adding the sphinx-lint tool to archery and our pre-commit hooks. In both locations, only two rules are enabled at the moment (Discussed in #40006): `trailing-whitespace` and `missing-final-newline`.

This PR also fixes the individual issues covered by the new tooling. 

### Are these changes tested?

Yes, though manually. I tested this works by running `archery lint --docs` and `pre-commit` without and without changes that should get caught by the rules. It works as expected.

### Are there any user-facing changes?

Yes,

1. Developers that use pre-commit hooks will see a change in behavior when they modify docs
2. Developers using archery will see a new --docs option in `archery lint`
3. Developers working on the docs may see CI failures related to the new checks

* Closes: #39990
* GitHub Issue: #39990

Authored-by: Bryce Mecum <petridish@gmail.com>
Signed-off-by: Bryce Mecum <petridish@gmail.com>
vibhatha pushed a commit to vibhatha/arrow that referenced this pull request May 25, 2024
…0022)

### What changes are included in this PR?

This adds developer tooling to the repo for linting the docs by adding the sphinx-lint tool to archery and our pre-commit hooks. In both locations, only two rules are enabled at the moment (Discussed in apache#40006): `trailing-whitespace` and `missing-final-newline`.

This PR also fixes the individual issues covered by the new tooling. 

### Are these changes tested?

Yes, though manually. I tested this works by running `archery lint --docs` and `pre-commit` without and without changes that should get caught by the rules. It works as expected.

### Are there any user-facing changes?

Yes,

1. Developers that use pre-commit hooks will see a change in behavior when they modify docs
2. Developers using archery will see a new --docs option in `archery lint`
3. Developers working on the docs may see CI failures related to the new checks

* Closes: apache#39990
* GitHub Issue: apache#39990

Authored-by: Bryce Mecum <petridish@gmail.com>
Signed-off-by: Bryce Mecum <petridish@gmail.com>
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.

3 participants