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

Savage beast tweaks #2249

Merged
merged 3 commits into from Mar 28, 2024
Merged

Savage beast tweaks #2249

merged 3 commits into from Mar 28, 2024

Conversation

Veracity0
Copy link
Contributor

  1. Cannot uneffect Mild-Mannered Professor and Savage Beast intrinsics
  2. Savage Beast cannot use NPC stores.

@Veracity0 Veracity0 requested a review from a team as a code owner March 28, 2024 15:55
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.58%. Comparing base (81ddf4f) to head (165c52a).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2249      +/-   ##
============================================
- Coverage     37.58%   37.58%   -0.01%     
  Complexity    19900    19900              
============================================
  Files          1110     1110              
  Lines        169699   169701       +2     
  Branches      35842    35843       +1     
============================================
- Hits          63787    63783       -4     
- Misses        95812    95817       +5     
- Partials      10100    10101       +1     
Files Coverage Δ
...ceforge/kolmafia/persistence/NPCStoreDatabase.java 45.83% <100.00%> (+0.37%) ⬆️
.../sourceforge/kolmafia/request/UneffectRequest.java 47.56% <ø> (ø)

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

@Veracity0 Veracity0 merged commit 19992da into main Mar 28, 2024
8 checks passed
@Veracity0 Veracity0 deleted the savage-beast-tweaks branch March 28, 2024 19:20
@Veracity0
Copy link
Contributor Author

Veracity0 commented Mar 29, 2024

In case you are wondering.

  1. I submitted this PR which makes two quality of life improvements for every WereProfessor run.
  2. It includes tests which give 100% coverage.
  3. I wait for approval.
  4. Two hours later, gausie makes a PR with code that helps only him.
  5. It includes tests which give 100% coverage.
  6. It is immediately approved.
  7. My PR, which helps every WereProfessor player, sits there unapproved.
  8. I wait a long time. No action.
  9. I merged without approval.

What the fuck happened?

Gausie could have approved my PR before submitting his PR, but does he read/approve PRs anymore, or only submit them?

My personal policy is that I only look at PRs from people who look at MY PRs. You are either part of the dev community or you are not.

@gausie
Copy link
Contributor

gausie commented Mar 29, 2024

I made that PR quickly in response to ask observation I made while communicating directly with Ryo. I actually made it entirely from the command line, so didn’t even see any waiting PRs in passing. Yesterday was my first day back at my job since taking 3 months of paternity leave which frees me up a little for contributing here, something I’ve otherwise struggled to do on a cycle of baby duty and recovery time. I’m looking forward to reviewing PRs again, though I have done so on a few occasions during my mini absence. I suppose the last point is that my PR doesn’t just help “only” me, no need to include something so obviously incorrect when expressing how you feel.

@Veracity0
Copy link
Contributor Author

I apologize for my tone and obvious incorrectness.
Welcome back.

@midgleyc
Copy link
Member

Gausie's PR looked trivial, so I scanned it over and approved it; this one looked a bit more complicated, so I was going to get to it later.

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