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

Improve documentation of JoinFeature #34508

Closed
soehms opened this issue Sep 8, 2022 · 13 comments
Closed

Improve documentation of JoinFeature #34508

soehms opened this issue Sep 8, 2022 · 13 comments

Comments

@soehms
Copy link
Member

soehms commented Sep 8, 2022

As observed in #34282 (see comment 28 ff there) there are two different use cases of this class which need to be explained in the doc-string.

Component: documentation

Keywords: join feature

Author: Sebastian Oehms

Branch/Commit: 68d4fe2

Reviewer: Kwankyu Lee

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

@soehms soehms added this to the sage-9.8 milestone Sep 8, 2022
@soehms
Copy link
Member Author

soehms commented Sep 8, 2022

Branch: u/soehms/docu_join_feature_34508

@soehms
Copy link
Member Author

soehms commented Sep 8, 2022

New commits:

a4768fa34508: initial

@soehms
Copy link
Member Author

soehms commented Sep 8, 2022

Commit: a4768fa

@soehms
Copy link
Member Author

soehms commented Sep 8, 2022

Author: Sebastian Oehms

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 9, 2022

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

ff38871Merge branch 'u/soehms/docu_join_feature_34508' of trac.sagemath.org:sage into docu_join_feature_34508
23d9d6c34508: typo

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 9, 2022

Changed commit from a4768fa to 23d9d6c

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 9, 2022

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 9, 2022

Changed commit from 23d9d6c to 68d4fe2

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 9, 2022

comment:5

I made minor edits. In particular, I removed the link to the ticket since the docstring summarizes important points of the discussion in the ticket. (I don't like links to trac tickets in our documentation except obviously necessary cases.)

If you are okay, you can set positive-review.


New commits:

68d4fe2Some edits

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 9, 2022

Reviewer: Kwankyu Lee

@soehms
Copy link
Member Author

soehms commented Sep 9, 2022

comment:7

Replying to Kwankyu Lee:

I made minor edits. In particular, I removed the link to the ticket since the docstring summarizes important points of the discussion in the ticket. (I don't like links to trac tickets in our documentation except obviously necessary cases.)

I don't like them in parts of the reference manual dedicated to end users, either. But here I didn't mind about it.

If you are okay, you can set positive-review.

Anyway, I'm fine with your changes. Thank you!

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 9, 2022

comment:8

Replying to Sebastian Oehms:

I don't like them (links to trac tickets) in parts of the reference manual dedicated to end users, either.

Exactly. In this case, end users are developers :)

Anyway, I'm fine with your changes. Thank you!

Thanks!

@vbraun
Copy link
Member

vbraun commented Sep 27, 2022

Changed branch from u/klee/docu_join_feature_34508 to 68d4fe2

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

3 participants