Skip to content

Add issue and PR templates#34983

Merged
dimpase merged 2 commits intosagemath:developfrom
tobiasdiez:add-issue-templates
Feb 8, 2023
Merged

Add issue and PR templates#34983
dimpase merged 2 commits intosagemath:developfrom
tobiasdiez:add-issue-templates

Conversation

@tobiasdiez
Copy link
Copy Markdown
Contributor

@tobiasdiez tobiasdiez commented Feb 6, 2023 •
edited
Loading

📚 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

@tobiasdiez tobiasdiez requested review from dimpase and mkoeppe February 6, 2023 15:05
@dimpase
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

tobiasdiez commented Feb 7, 2023 •
edited
Loading

@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

@tobiasdiez tobiasdiez mentioned this pull request Feb 7, 2023
Merged
dimpase
dimpase approved these changes Feb 7, 2023
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

roed314 commented Feb 7, 2023

Lgtm; just fixed a typo.

tobiasdiez and others added 2 commits February 7, 2023 20:25
@tobiasdiez @dimpase
Add issue and PR templates
ec0850d 
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>
@roed314 @dimpase
Fix typo
46cdc6c
@dimpase dimpase force-pushed the add-issue-templates branch from b866ac5 to 46cdc6c Compare February 7, 2023 20:25
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 8, 2023

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
Copy Markdown
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
Copy Markdown
Contributor Author

tobiasdiez commented Feb 8, 2023 •
edited
Loading

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
Copy Markdown
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 tobiasdiez mentioned this pull request Feb 8, 2023
Merged
4 tasks
@tobiasdiez
Copy link
Copy Markdown
Contributor Author

tobiasdiez commented Feb 8, 2023

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
Copy Markdown
Member

mezzarobba commented Feb 11, 2023

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
Copy Markdown
Contributor Author

tobiasdiez commented Feb 11, 2023 •
edited
Loading

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
Copy Markdown
Member

mezzarobba commented Feb 11, 2023

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

@tobiasdiez tobiasdiez mentioned this pull request Feb 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@dimpase dimpase dimpase approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

p: critical / 2

Projects

None yet

Milestone

sage-9.8

Development

Successfully merging this pull request may close these issues.

Set up Issue Templates, PR Templates

7 participants

@tobiasdiez @dimpase @roed314 @codecov-commenter @mezzarobba @kwankyu @mkoeppe