Skip to content

Conversation

kwankyu
Copy link
Collaborator

@kwankyu kwankyu commented Oct 19, 2023

Remove all spurious and chronic diffs in the changes of our documentation preview.

Improve CHANGES.html using diffsite. See https://deploy-preview-36483--sagemath-tobias.netlify.app/changes

diffsite allows to compare changes visually. Scroll until vertical alignment breaks or search for some diff text.

In this regard, refrain from using a function as the default value of an argument in Python definitions. Use None instead and apply the default (the function) in the code part.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@kwankyu kwankyu changed the title remove spurious doc diffs Remove spurious diffs in doc build changes Oct 19, 2023
@kwankyu kwankyu force-pushed the remove-spurious-doc-diffs branch from 7827a10 to 2c04ca0 Compare October 20, 2023 08:21
@kwankyu kwankyu force-pushed the remove-spurious-doc-diffs branch from 349da52 to b5b49e2 Compare October 20, 2023 12:56
@kwankyu kwankyu force-pushed the remove-spurious-doc-diffs branch from 9e5dae7 to 228cb67 Compare October 20, 2023 21:28
@kwankyu kwankyu marked this pull request as ready for review October 20, 2023 22:32
@kwankyu kwankyu force-pushed the remove-spurious-doc-diffs branch from 0cbd08c to 3013bd5 Compare October 20, 2023 22:55
@kwankyu
Copy link
Collaborator Author

kwankyu commented Oct 20, 2023

Thanks. But I found a glitch meanwhile. Let me fix this first :-)

@kwankyu kwankyu force-pushed the remove-spurious-doc-diffs branch from bc4176e to c81d50f Compare October 21, 2023 05:03
@kwankyu
Copy link
Collaborator Author

kwankyu commented Oct 21, 2023

Ready to go. Thanks!

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 21, 2023

needs rebase now

@kwankyu
Copy link
Collaborator Author

kwankyu commented Oct 21, 2023

Thanks. Merged with develop cleanly by git.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Oct 21, 2023

By the way, is there a reason that changes are only from html/en, that is, English doc, instead of html?

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 21, 2023

Only the English docs are uploaded to netlify, see the end of the "Copy docs" step.
I don't recall if this was discussed when the netlify deployment was developed.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Oct 21, 2023

Ah, right.

Then this is a bigger problem, to be discussed elsewhere. Thanks.

@github-actions
Copy link

Documentation preview for this PR (built with commit 67e7f95; changes) is ready! 🎉

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 21, 2023
sagemathgh-36483: Remove spurious diffs in doc build changes
    
Remove all spurious and chronic diffs in the changes of our
documentation preview.

Improve CHANGES.html using diffsite. See https://deploy-preview-36483--
sagemath-tobias.netlify.app/changes

diffsite allows to compare changes visually. Scroll until vertical
alignment breaks or search for some diff text.

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

- sagemath#36475

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36483
Reported by: Kwankyu Lee
Reviewer(s): Matthias Köppe
@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 22, 2023

The improved CHANGES display is really nice.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Oct 22, 2023

Thanks. It would've been better if diffsite scrolls to (or highlight) diff parts. I found no way to do that automatically (perhaps need AI). But from my experience, the manual method in the description of this PR works well enough.

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 24, 2023
sagemathgh-36483: Remove spurious diffs in doc build changes
    
Remove all spurious and chronic diffs in the changes of our
documentation preview.

Improve CHANGES.html using diffsite. See https://deploy-preview-36483--
sagemath-tobias.netlify.app/changes

diffsite allows to compare changes visually. *Scroll until vertical
alignment breaks or search for some diff text.*

In this regard, refrain from using a function as the default value of an
argument in Python definitions. Use `None` instead and apply the default
(the function) in the code part.

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

- sagemath#36475

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36483
Reported by: Kwankyu Lee
Reviewer(s): Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 26, 2023
sagemathgh-36483: Remove spurious diffs in doc build changes
    
Remove all spurious and chronic diffs in the changes of our
documentation preview.

Improve CHANGES.html using diffsite. See https://deploy-preview-36483--
sagemath-tobias.netlify.app/changes

diffsite allows to compare changes visually. *Scroll until vertical
alignment breaks or search for some diff text.*

In this regard, refrain from using a function as the default value of an
argument in Python definitions. Use `None` instead and apply the default
(the function) in the code part.

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

- sagemath#36475

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36483
Reported by: Kwankyu Lee
Reviewer(s): Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 28, 2023
sagemathgh-36483: Remove spurious diffs in doc build changes
    
Remove all spurious and chronic diffs in the changes of our
documentation preview.

Improve CHANGES.html using diffsite. See https://deploy-preview-36483--
sagemath-tobias.netlify.app/changes

diffsite allows to compare changes visually. *Scroll until vertical
alignment breaks or search for some diff text.*

In this regard, refrain from using a function as the default value of an
argument in Python definitions. Use `None` instead and apply the default
(the function) in the code part.

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

- sagemath#36475

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36483
Reported by: Kwankyu Lee
Reviewer(s): Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 28, 2023
sagemathgh-36483: Remove spurious diffs in doc build changes
    
Remove all spurious and chronic diffs in the changes of our
documentation preview.

Improve CHANGES.html using diffsite. See https://deploy-preview-36483--
sagemath-tobias.netlify.app/changes

diffsite allows to compare changes visually. *Scroll until vertical
alignment breaks or search for some diff text.*

In this regard, refrain from using a function as the default value of an
argument in Python definitions. Use `None` instead and apply the default
(the function) in the code part.

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

- sagemath#36475

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36483
Reported by: Kwankyu Lee
Reviewer(s): Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 29, 2023
sagemathgh-36483: Remove spurious diffs in doc build changes
    
Remove all spurious and chronic diffs in the changes of our
documentation preview.

Improve CHANGES.html using diffsite. See https://deploy-preview-36483--
sagemath-tobias.netlify.app/changes

diffsite allows to compare changes visually. *Scroll until vertical
alignment breaks or search for some diff text.*

In this regard, refrain from using a function as the default value of an
argument in Python definitions. Use `None` instead and apply the default
(the function) in the code part.

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

- sagemath#36475

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36483
Reported by: Kwankyu Lee
Reviewer(s): Matthias Köppe
@vbraun vbraun merged commit 83dcc17 into sagemath:develop Oct 31, 2023
@kwankyu kwankyu deleted the remove-spurious-doc-diffs branch December 13, 2023 11:48
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