Opened 6 years ago
Closed 6 years ago
#22160 closed enhancement (fixed)
Use centralized references in ClusterSeed
Reported by: | etn40ff | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.6 |
Component: | documentation | Keywords: | documentation, days82 |
Cc: | gmoose05, drupel, stumpc5, tscrim, egunawan | Merged in: | |
Authors: | Salvatore Stella | Reviewers: | Emily Gunawan, Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | b787c1c (Commits, GitHub, GitLab) | Commit: | b787c1cfa8303a5302fe3998eaa270a2126ac9fe |
Dependencies: | Stopgaps: |
Description (last modified by )
While reviewing #21254 egunawan found a double reference due to the fact that ClusterSeed
still has references defined locally.
This ticket fixes the issue.
Change History (28)
comment:1 Changed 6 years ago by
- Branch set to public/ticket/22160
- Commit set to 04f4244474d651adc9eacfa1303bba31e591900f
comment:2 Changed 6 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:3 Changed 6 years ago by
- Commit changed from 04f4244474d651adc9eacfa1303bba31e591900f to 175b53aa7054b9483f12747da3b27dc4b8ee32af
Branch pushed to git repo; I updated commit sha1. New commits:
175b53a | I can\'t even remember the lexicographic order!
|
comment:4 Changed 6 years ago by
- Keywords days82 added
- Status changed from needs_review to needs_info
There are several more local references in cluster_seed.py. We should move them to index.rst as well?
comment:5 Changed 6 years ago by
I think we should just move them all at once.
comment:6 Changed 6 years ago by
Agreed, I did not notice them. I'll work on it in one hour or so.
comment:7 Changed 6 years ago by
- Commit changed from 175b53aa7054b9483f12747da3b27dc4b8ee32af to 976b736920f2ddbeb473440d7e71d304e0d885fa
Branch pushed to git repo; I updated commit sha1. New commits:
976b736 | Moved all references
|
comment:8 Changed 6 years ago by
- Status changed from needs_info to needs_review
comment:9 Changed 6 years ago by
- Commit changed from 976b736920f2ddbeb473440d7e71d304e0d885fa to 474750e709dfb6d23b5eb99d856690fa4e74c4fb
Branch pushed to git repo; I updated commit sha1. New commits:
474750e | Added missing references
|
comment:10 Changed 6 years ago by
There is a typo but I am recompiling on a different branch and I can't fix it right now. I should be able to mend the problem as soon as the documentation is done building.
comment:11 Changed 6 years ago by
- Commit changed from 474750e709dfb6d23b5eb99d856690fa4e74c4fb to b3ced177ba29c0b81c3bc0b6071fdc7d5e27ac9d
Branch pushed to git repo; I updated commit sha1. New commits:
b3ced17 | Fix typo
|
comment:12 Changed 6 years ago by
Fixed, it should be good now.
comment:13 Changed 6 years ago by
- Component changed from PLEASE CHANGE to documentation
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
LGTM.
comment:14 Changed 6 years ago by
- Milestone changed from sage-7.5 to sage-7.6
- Reviewers changed from Travis Scrimshaw to Emily Gunawan, Travis Scrimshaw
comment:15 Changed 6 years ago by
- Status changed from positive_review to needs_info
There are other references to actual websites (or arxiv numbers) in cluster_seed.py and other parts of the cluster_algebra_quiver folder. This will not cause issue for Sage, but, since this ticket was already opened, should we move those local references as well? If you don't think we should do that, you can set this back to positive_review.
comment:16 Changed 6 years ago by
- Commit changed from b3ced177ba29c0b81c3bc0b6071fdc7d5e27ac9d to 29cc90a37869d74e20f5f5c05a4c04a5812b3b2c
comment:17 Changed 6 years ago by
- Status changed from needs_info to needs_review
I now changed also all the references in every file under the folder cluster_algebra_quiver. They were in a messy variety of different styles.
comment:18 Changed 6 years ago by
- Status changed from needs_review to needs_work
We should be good and make sure those references have full bib information. For example, I suspect the Speyer reference has been published in a journal.
comment:19 Changed 6 years ago by
I already did that. I could not find better references for [Spe2013]_ nor for [Ke2008]_
comment:20 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:21 Changed 6 years ago by
Okay, thanks. I will let Emily set it back to a positive review of there is nothing else.
comment:22 Changed 6 years ago by
- Commit changed from 29cc90a37869d74e20f5f5c05a4c04a5812b3b2c to c71e227f6808318cc36feab49c2b0ed1ad8b009d
Branch pushed to git repo; I updated commit sha1. New commits:
c71e227 | 22160: fix broken reference to see sage.graphs.generic_graph.GenericGraph.canonical_label()
|
comment:23 Changed 6 years ago by
Salvatore, you can set this to positive_review if you're OK with the change I just did (fixing a broken reference to GenericGraph?.canonical_label).
comment:26 Changed 6 years ago by
- Commit changed from c71e227f6808318cc36feab49c2b0ed1ad8b009d to b787c1cfa8303a5302fe3998eaa270a2126ac9fe
Branch pushed to git repo; I updated commit sha1. New commits:
49dc8b8 | Use centralized references
|
318d348 | I can\'t even remember the lexicographic order!
|
71149f7 | Moved all references
|
dbf7100 | Added missing references
|
591fbc0 | Fix typo
|
2b395ee | Changed all the remaining references in the cluster_algebra_quiver folder
|
4a0c541 | Minor fixes
|
be62a45 | 22160: fix broken reference to see sage.graphs.generic_graph.GenericGraph.canonical_label()
|
b787c1c | Fixed merge conflict
|
comment:27 Changed 6 years ago by
- Status changed from needs_work to positive_review
I rebased the branch to the current develop. This and some other ticket introduced two new references in the same place generating the conflict. It should be good now.
comment:28 Changed 6 years ago by
- Branch changed from public/ticket/22160 to b787c1cfa8303a5302fe3998eaa270a2126ac9fe
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Use centralized references