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:

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

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

New commits:

04f4244Use centralized references

comment:2 Changed 6 years ago by etn40ff

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

comment:3 Changed 6 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 6 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 6 years ago by tscrim

I think we should just move them all at once.

comment:6 Changed 6 years ago by etn40ff

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

comment:7 Changed 6 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 6 years ago by etn40ff

  • Status changed from needs_info to needs_review

comment:9 Changed 6 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 6 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 6 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 6 years ago by etn40ff

Fixed, it should be good now.

comment:13 Changed 6 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 6 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 6 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?

Version 0, edited 6 years ago by egunawan (next)

comment:16 Changed 6 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 6 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 6 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 6 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 6 years ago by etn40ff (previous) (diff)

comment:20 Changed 6 years ago by etn40ff

  • Status changed from needs_work to needs_review

comment:21 Changed 6 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 6 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 6 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 6 years ago by etn40ff

  • Status changed from needs_review to positive_review

LGTM

comment:25 Changed 6 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:26 Changed 6 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 6 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 6 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.