Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typo in the documentation of weierstrass_points #11798

Closed
zimmermann6 opened this issue Sep 14, 2011 · 12 comments
Closed

typo in the documentation of weierstrass_points #11798

zimmermann6 opened this issue Sep 14, 2011 · 12 comments

Comments

@zimmermann6
Copy link

sage: K = pAdicField(11, 5)
sage: x = polygen(K)
sage: C = HyperellipticCurve(x^5 + 1)
sage: C.weierstrass_points?
...
       in the suport of the divisor of $y$

Here, suport should be support.


Apply attachment: trac11798.patch to the Sage library.

CC: @defeo

Component: documentation

Keywords: ecc2011

Author: Paul Zimmermann

Reviewer: Luca De Feo

Merged: sage-4.7.2.alpha3

Issue created by migration from https://trac.sagemath.org/ticket/11798

@zimmermann6
Copy link
Author

Attachment: trac11798.patch.gz

@zimmermann6
Copy link
Author

comment:1

The attached patch fixes that critical bug.

@zimmermann6
Copy link
Author

Author: Paul Zimmermann

@defeo
Copy link
Member

defeo commented Sep 14, 2011

comment:2

Wonderful spell checking!

@defeo
Copy link
Member

defeo commented Sep 14, 2011

Reviewer: Luca De Feo

@zimmermann6
Copy link
Author

Changed keywords from none to ecc2011

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 16, 2011

Replying to @zimmermann6:

Note: I have created that ticket to show how to create a patch and
submit it to trac during the ECC 2011 summer school. Thus please don't
work on this ticket!

Sorry, Paul, I have to, since the patch to be applied is not mentioned in the ticket's description, which is good practice, and also necessary for at least some bots or release tools which partially automate the process. (In case there are multiple patches which have to be applied, they should be listed in the order in which they have to be applied.)


To potentially new contributors / Sage developers:

It isn't bad to choose filenames that at least partially reflect what a patch does, or to which component / defect etc. it belongs; also, using the comment field of attachments isn't bad, e.g. to note to which repository a patch has to be applied, which version of Sage it was based on, and maybe also there what a patch does, etc.

As Paul did, the filenames of patches should start with trac_<ticket number> (or, less often used, trac<ticket number>), and the commit message should contain (best start with) the ticket number on its first line, e.g. #11798 ... or Trac 11798: ... or the like.

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 16, 2011

comment:5

P.S.: Some might consider this redundant (at least in cases where there's only one patch, and which has to be applied to the "default" repository, i.e., the one of the Sage libary), but a little redundancy is good to avoid errors when automating things.

If there are multiple patches attached to a ticket, listing them (or just the single proper one to be applied) in the correct order in the ticket's description is IMHO mandatory, since humans would also like to immediately have this information (as opposed to reading and reasoning about potentially many comments on the ticket).

Also, the URLs of new / updated spkgs provided by a ticket should always be part of the ticket's description.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 17, 2011

Merged: sage-4.7.2.alpha3

@nexttime nexttime mannequin removed the s: positive review label Sep 17, 2011
@nexttime nexttime mannequin closed this as completed Sep 17, 2011
@zimmermann6

This comment has been minimized.

@zimmermann6
Copy link
Author

comment:7

thank you Leif for your comments! I forgot to update the description after my demonstration.

Paul

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants