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

Add issue and PR templates #34983

Merged
merged 2 commits into from
Feb 8, 2023
Merged

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Feb 6, 2023

📚 Description

Improved version based on https://github.com/sagemath/sage-gh-templates-sandbox
Fixes sagemath/trac-to-github#86.

📝 Checklist

  • I have linked an issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@dimpase
Copy link
Member

dimpase commented Feb 6, 2023

@tobiasdiez - can you enable issues and PRs on your fork, just to see how this looks like in production?

@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented Feb 7, 2023

@dimpase Sure, good idea, should have done this directly. For the PR template, this PR already uses it and you can test it now at tobiasdiez/sage@develop...tobiasdiez:sage:fix-netlify. For a new issue, have a look at https://github.com/tobiasdiez/sage/issues/new/choose

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@dimpase dimpase removed the request for review from mkoeppe February 7, 2023 17:19
@dimpase
Copy link
Member

dimpase commented Feb 7, 2023

I took the liberty to remove @mkoeppe from the reviewers list - just to streamline this one.

@roed314
Copy link
Contributor

roed314 commented Feb 7, 2023

Lgtm; just fixed a typo.

tobiasdiez and others added 2 commits February 7, 2023 20:25
Improved version based on https://github.com/sagemath/sage-gh-templates-sandbox
Fixes sagemath/trac-to-github#86.

Co-authored-by: Matthias Köppe <mkoeppe@math.ucdavis.edu>
@codecov-commenter
Copy link

Codecov Report

Base: 88.59% // Head: 88.59% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (46cdc6c) compared to base (66b319b).
Patch has no changes to coverable lines.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #34983      +/-   ##
===========================================
- Coverage    88.59%   88.59%   -0.01%     
===========================================
  Files         2136     2136              
  Lines       396141   396141              
===========================================
- Hits        350948   350943       -5     
- Misses       45193    45198       +5     
Impacted Files Coverage Δ
src/sage/crypto/util.py 91.37% <0.00%> (-5.18%) ⬇️
src/sage/interfaces/qsieve.py 71.30% <0.00%> (-2.61%) ⬇️
src/sage/modular/local_comp/liftings.py 98.33% <0.00%> (-1.67%) ⬇️
src/sage/graphs/generators/random.py 91.09% <0.00%> (-1.55%) ⬇️
src/sage/schemes/elliptic_curves/hom_frobenius.py 96.34% <0.00%> (-1.22%) ⬇️
src/sage/schemes/elliptic_curves/hom_velusqrt.py 94.48% <0.00%> (-0.79%) ⬇️
src/sage/coding/channel.py 97.10% <0.00%> (-0.73%) ⬇️
src/sage/graphs/graph_plot.py 84.33% <0.00%> (-0.57%) ⬇️
src/sage/modular/arithgroup/congroup_gamma0.py 94.41% <0.00%> (-0.56%) ⬇️
src/sage/sets/integer_range.py 91.41% <0.00%> (-0.51%) ⬇️
... and 22 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dimpase
Copy link
Member

dimpase commented Feb 8, 2023

I'm merging this too - it makes creation of PRs and issues more meaningful and documented. Nothing else is affected.

@dimpase dimpase merged commit 6a4667b into sagemath:develop Feb 8, 2023
@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented Feb 8, 2023

Thanks!

Do you think we should also give hints for good PR titles, e.g. "Fix build error of scipy" instead of Fixes #<number>?

@dimpase
Copy link
Member

dimpase commented Feb 8, 2023

Do you think we should also give hints for good PR titles, e.g. "Fix build error of scipy" instead of Fixes #<number>?

indeed, can you fix this?

@tobiasdiez
Copy link
Contributor Author

Sure: #35022

By the way, if you "squash" a PR instead of merging it, you normally get a nicer more linear commit history in the develop branch.

@tobiasdiez tobiasdiez deleted the add-issue-templates branch February 8, 2023 10:46
@mezzarobba
Copy link
Member

Some approved PRs, e.g., #35064, don't show as such in the list. I wonder if this is because of the checklist in the PR template, which github interprets as a list of tasks.

@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented Feb 11, 2023

You mean the missing "Approved" in the list? This comes from the fact that github only takes reviews from people with write/maintainer rights into account when computing that. And in #35064, Travis has "only" triage rights. This is also the reason why his review is only a gray tick, while Matthias review at #35060 is a green tick.

@mezzarobba
Copy link
Member

Yes, that's what I meant. Thanks for the explanation, and sorry for the noise.

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

Successfully merging this pull request may close these issues.

Set up Issue Templates, PR Templates
7 participants