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

Unbreak the build (fix #586 #597) #595

Merged
merged 7 commits into from
Jan 14, 2021
Merged

Conversation

sloshy
Copy link
Contributor

@sloshy sloshy commented Jan 12, 2021

Fixes the build by solving two separate issues:

  1. Ignores a failing test that was deemed safe to ignore (Ignore the reviewers test #586)
  2. Removes the Auth.newAuth functionality as logging in with a username/password is now officially discontinued (Remove Auth.newAuth as it has been discontinued #597)

This includes the requisite changes to documentation as well as a quick tweak to the .gitignore to properly filter out metals.sbt in all directories.

@github-actions github-actions bot added the bug Something isn't working label Jan 12, 2021
@sloshy sloshy changed the title Ignore PullRequests reviewers test (fix #586) WIP: Ignore PullRequests reviewers test (fix #586) Jan 12, 2021
@sloshy
Copy link
Contributor Author

sloshy commented Jan 12, 2021

Marked WIP as there seems to be another test failing. Not sure if it should hold up the fix.

@sloshy sloshy marked this pull request as draft January 12, 2021 19:18
@sloshy sloshy marked this pull request as ready for review January 13, 2021 13:30
@sloshy sloshy changed the title WIP: Ignore PullRequests reviewers test (fix #586) Ignore PullRequests reviewers test (fix #586) Jan 13, 2021
@sloshy sloshy changed the title Ignore PullRequests reviewers test (fix #586) Unbreak the build (fix #586 #597) Jan 13, 2021
@sloshy sloshy force-pushed the fix/586-ignore-reviewers-test branch from 9188177 to 09a3b4d Compare January 13, 2021 17:37
@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #595 (09a3b4d) into master (fbb265a) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #595      +/-   ##
==========================================
- Coverage   90.29%   90.18%   -0.12%     
==========================================
  Files          25       25              
  Lines         608      611       +3     
  Branches        3        1       -2     
==========================================
+ Hits          549      551       +2     
- Misses         59       60       +1     
Impacted Files Coverage Δ
...s/src/main/scala/github4s/domain/PullRequest.scala 100.00% <ø> (ø)
.../scala/github4s/interpreters/AuthInterpreter.scala 100.00% <ø> (ø)
github4s/src/main/scala/github4s/Decoders.scala 91.91% <100.00%> (ø)
github4s/src/main/scala/github4s/Encoders.scala 96.77% <100.00%> (ø)
...ithub4s/interpreters/PullRequestsInterpreter.scala 100.00% <100.00%> (ø)
...ub4s/src/main/scala/github4s/http/HttpClient.scala 92.68% <0.00%> (-2.44%) ⬇️

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 df1a16f...09a3b4d. Read the comment docs.

@sloshy
Copy link
Contributor Author

sloshy commented Jan 13, 2021

Ok, after a lot of trial and error (Metals was not reporting any of the errors that github found) I've turned this PR into fixing the build entirely, and edited the description to match. Code coverage is slightly down but I don't think that's an issue since it's a result of one test being removed (as the feature was removed) and then one test being ignored (as the issue states).

Copy link
Contributor

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

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

👍

@BenFradet BenFradet merged commit fd7154e into master Jan 14, 2021
@BenFradet BenFradet deleted the fix/586-ignore-reviewers-test branch January 14, 2021 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants