Opened 4 years ago

Closed 4 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:

Status badges

Description (last modified by etn40ff)

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 4 years ago by etn40ff

  • Branch set to public/ticket/22160
  • Commit set to 04f4244474d651adc9eacfa1303bba31e591900f

New commits:

04f4244Use centralized references

comment:2 Changed 4 years ago by etn40ff

  • Description modified (diff)
  • Status changed from new to needs_review

comment:3 Changed 4 years ago by git

  • Commit changed from 04f4244474d651adc9eacfa1303bba31e591900f to 175b53aa7054b9483f12747da3b27dc4b8ee32af

Branch pushed to git repo; I updated commit sha1. New commits:

175b53aI can\'t even remember the lexicographic order!

comment:4 Changed 4 years ago by egunawan

  • 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 4 years ago by tscrim

I think we should just move them all at once.

comment:6 Changed 4 years ago by etn40ff

Agreed, I did not notice them. I'll work on it in one hour or so.

comment:7 Changed 4 years ago by git

  • Commit changed from 175b53aa7054b9483f12747da3b27dc4b8ee32af to 976b736920f2ddbeb473440d7e71d304e0d885fa

Branch pushed to git repo; I updated commit sha1. New commits:

976b736Moved all references

comment:8 Changed 4 years ago by etn40ff

  • Status changed from needs_info to needs_review

comment:9 Changed 4 years ago by git

  • Commit changed from 976b736920f2ddbeb473440d7e71d304e0d885fa to 474750e709dfb6d23b5eb99d856690fa4e74c4fb

Branch pushed to git repo; I updated commit sha1. New commits:

474750eAdded missing references

comment:10 Changed 4 years ago by etn40ff

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 4 years ago by git

  • Commit changed from 474750e709dfb6d23b5eb99d856690fa4e74c4fb to b3ced177ba29c0b81c3bc0b6071fdc7d5e27ac9d

Branch pushed to git repo; I updated commit sha1. New commits:

b3ced17Fix typo

comment:12 Changed 4 years ago by etn40ff

Fixed, it should be good now.

comment:13 Changed 4 years ago by tscrim

  • Component changed from PLEASE CHANGE to documentation
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:14 Changed 4 years ago by tscrim

  • Milestone changed from sage-7.5 to sage-7.6
  • Reviewers changed from Travis Scrimshaw to Emily Gunawan, Travis Scrimshaw

comment:15 Changed 4 years ago by egunawan

  • 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.

Last edited 4 years ago by egunawan (previous) (diff)

comment:16 Changed 4 years ago by git

  • Commit changed from b3ced177ba29c0b81c3bc0b6071fdc7d5e27ac9d to 29cc90a37869d74e20f5f5c05a4c04a5812b3b2c

Branch pushed to git repo; I updated commit sha1. New commits:

0d5dfb6Changed all the remaining references in the cluster_algebra_quiver folder
29cc90aMinor fixes

comment:17 Changed 4 years ago by etn40ff

  • 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 4 years ago by tscrim

  • 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 4 years ago by etn40ff

I already did that. I could not find better references for [Spe2013]_ nor for [Ke2008]_. I did fix many of the others.

Last edited 4 years ago by etn40ff (previous) (diff)

comment:20 Changed 4 years ago by etn40ff

  • Status changed from needs_work to needs_review

comment:21 Changed 4 years ago by tscrim

Okay, thanks. I will let Emily set it back to a positive review of there is nothing else.

comment:22 Changed 4 years ago by git

  • Commit changed from 29cc90a37869d74e20f5f5c05a4c04a5812b3b2c to c71e227f6808318cc36feab49c2b0ed1ad8b009d

Branch pushed to git repo; I updated commit sha1. New commits:

c71e22722160: fix broken reference to see sage.graphs.generic_graph.GenericGraph.canonical_label()

comment:23 Changed 4 years ago by egunawan

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:24 Changed 4 years ago by etn40ff

  • Status changed from needs_review to positive_review

LGTM

comment:25 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:26 Changed 4 years ago by git

  • Commit changed from c71e227f6808318cc36feab49c2b0ed1ad8b009d to b787c1cfa8303a5302fe3998eaa270a2126ac9fe

Branch pushed to git repo; I updated commit sha1. New commits:

49dc8b8Use centralized references
318d348I can\'t even remember the lexicographic order!
71149f7Moved all references
dbf7100Added missing references
591fbc0Fix typo
2b395eeChanged all the remaining references in the cluster_algebra_quiver folder
4a0c541Minor fixes
be62a4522160: fix broken reference to see sage.graphs.generic_graph.GenericGraph.canonical_label()
b787c1cFixed merge conflict

comment:27 Changed 4 years ago by etn40ff

  • 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 4 years ago by vbraun

  • Branch changed from public/ticket/22160 to b787c1cfa8303a5302fe3998eaa270a2126ac9fe
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.