Skip to content

Allow for deleting and restoring branch associated with PR#1784

Merged
EnricoMi merged 21 commits intoPyGithub:mainfrom
austinsasko:feature/adding-delete-restore-logic
Jul 25, 2024
Merged

Allow for deleting and restoring branch associated with PR#1784
EnricoMi merged 21 commits intoPyGithub:mainfrom
austinsasko:feature/adding-delete-restore-logic

Conversation

@austinsasko
Copy link
Copy Markdown
Contributor

@austinsasko austinsasko commented Dec 14, 2020 •
edited
Loading

Adding logic for deleting/restoring branches associated with a PR, as well as forcing a delete.
Fixes #580

@austinsasko austinsasko changed the title Fixes #580 Implementing logic for deleting and restoring a branch associated with a PR Dec 14, 2020
s-t-e-v-e-n-k
s-t-e-v-e-n-k requested changes Dec 16, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@s-t-e-v-e-n-k s-t-e-v-e-n-k left a comment

Choose a reason for hiding this comment

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

This is looking like a great start, I have some concerns I have outlined.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 24, 2020 •
edited
Loading

Codecov Report

Merging #1784 (16b2544) into master (34d097c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1784   +/-   ##
=======================================
  Coverage   98.76%   98.76%           
=======================================
  Files          51       51           
  Lines        2677     2677           
=======================================
  Hits         2644     2644           
  Misses         33       33

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34d097c...16b2544. Read the comment docs.

@austinsasko austinsasko force-pushed the feature/adding-delete-restore-logic branch 3 times, most recently from a254363 to fee99a2 Compare December 24, 2020 01:02
@austinsasko austinsasko force-pushed the feature/adding-delete-restore-logic branch from fee99a2 to f299699 Compare December 24, 2020 01:25
@austinsasko austinsasko reopened this Dec 24, 2020
@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Dec 24, 2020 •
edited
Loading

@s-t-e-v-e-n-k I am trying to figure out why code coverage decreased on a file that I did not interface with or modify any tests that interacted with it. Any ideas?

Update: It appears the changes in #1797 will bring the code coverage back to up to the place it should be. I believe the latest pyJWT being released yesterday is causing codecov to not reach that line in MainClass.py -- as long as that PR (1797) is merged before this one, I think that codecoverage wont be affected. Apologies for any spam that I caused by troubleshooting that.

@Seluj78 Seluj78 mentioned this pull request Dec 27, 2020
Closed
@austinsasko austinsasko force-pushed the feature/adding-delete-restore-logic branch from dd0718d to 16b2544 Compare December 28, 2020 19:35
@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Dec 28, 2020 •
edited
Loading

As believed, now that the other PR has been merged (and upon me pulling latest master into my branch) this one is ready to be merged as well with no issues :)
@s-t-e-v-e-n-k do you need anything else from me?

Thanks

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Feb 5, 2021

@s-t-e-v-e-n-k @sfdye Is there anything else in this PR needing changes?

s-t-e-v-e-n-k
s-t-e-v-e-n-k requested changes Mar 24, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@s-t-e-v-e-n-k s-t-e-v-e-n-k left a comment

Choose a reason for hiding this comment

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

This is looking like a good start, I have some concerns inline.

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Mar 26, 2021

@s-t-e-v-e-n-k 👍 applied your recommendations

s-t-e-v-e-n-k
s-t-e-v-e-n-k requested changes Mar 27, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@s-t-e-v-e-n-k s-t-e-v-e-n-k left a comment

Choose a reason for hiding this comment

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

Thanks for your changes, you're making good progress, some more changes required, though.

@austinsasko austinsasko deleted the feature/adding-delete-restore-logic branch March 29, 2021 17:32
@austinsasko austinsasko restored the feature/adding-delete-restore-logic branch March 29, 2021 17:33
@austinsasko austinsasko reopened this Mar 29, 2021
@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Apr 5, 2021

@s-t-e-v-e-n-k removing import github breaks the CI tests. I re-added it and the tests are passing.

@Seluj78
Copy link
Copy Markdown

Seluj78 commented Apr 24, 2021

Hi ! any news on this ? I'd love to implement it on https://github.com/AFPy/PyDocTeur :)

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented May 25, 2021

@s-t-e-v-e-n-k

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Jun 15, 2021

@sfdye anything we can do here?

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Dec 31, 2021

@s-t-e-v-e-n-k @sfdye

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Jan 16, 2022

@s-t-e-v-e-n-k @sfdye who maintains this project currently, if anyone?

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Apr 12, 2022

@s-t-e-v-e-n-k @sfdye hoping to make one last ditch effort to follow up

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Jun 27, 2024

No more conflicts :)

@EnricoMi @JLLeitschuh

@JLLeitschuh JLLeitschuh enabled auto-merge June 28, 2024 02:30
@JLLeitschuh JLLeitschuh disabled auto-merge June 28, 2024 02:30
@JLLeitschuh
Copy link
Copy Markdown
Collaborator

JLLeitschuh commented Jun 28, 2024

Ir looks like CI is failing

@JLLeitschuh
Copy link
Copy Markdown
Collaborator

JLLeitschuh commented Jun 28, 2024

Also, I don't know if .piy files are being used anymore. It would be good to check

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Jul 1, 2024

@JLLeitschuh I have resolved all CI checks, including one that was unrelated to this PR. I have also made all requested changes and re-requested all reviewers. Let me know if you need anything else

EnricoMi
EnricoMi requested changes Jul 2, 2024
View reviewed changes
github/PullRequest.pyi Outdated
@@ -0,0 +1,160 @@
from datetime import datetime
Copy link
Copy Markdown
Collaborator

@EnricoMi EnricoMi Jul 2, 2024

Choose a reason for hiding this comment

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

this file can be deleted

EnricoMi
EnricoMi requested changes Jul 2, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

move typing to .py file

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Jul 2, 2024

thanks for the feedback @EnricoMi -- fixed the remaining mypy issues and typing. Let me know if anything else is needed.

EnricoMi
EnricoMi requested changes Jul 3, 2024
View reviewed changes
@EnricoMi
Copy link
Copy Markdown
Collaborator

EnricoMi commented Jul 3, 2024

@s-t-e-v-e-n-k happy with this?

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Jul 18, 2024

@EnricoMi / @s-t-e-v-e-n-k what remains for this to get merged? Thanks!

JLLeitschuh
JLLeitschuh approved these changes Jul 18, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@JLLeitschuh JLLeitschuh left a comment

Choose a reason for hiding this comment

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

LGTM!

@austinsasko
Copy link
Copy Markdown
Contributor Author

austinsasko commented Jul 20, 2024

Now that we have two approvals and the changes that @s-t-e-v-e-n-k requested are completed, what is the process for merging? Thanks @JLLeitschuh and @EnricoMi

@EnricoMi EnricoMi merged commit 4ba1e41 into PyGithub:main Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JLLeitschuh JLLeitschuh JLLeitschuh approved these changes

@EnricoMi EnricoMi EnricoMi approved these changes

@s-t-e-v-e-n-k s-t-e-v-e-n-k Awaiting requested review from s-t-e-v-e-n-k

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Delete/restore branch associated with a pull request

8 participants

@austinsasko @codecov-io @Seluj78 @treee111 @codecov-commenter @JLLeitschuh @EnricoMi @s-t-e-v-e-n-k