-
-
Notifications
You must be signed in to change notification settings - Fork 164
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
use upstream hubcaps #624
use upstream hubcaps #624
Conversation
92478d0
to
f1abcd5
Compare
f1abcd5
to
1eeed03
Compare
# hyper = { version = "0.14", features = ["full"] } | ||
hyper = "=0.10.*" | ||
hyper-native-tls = "0.3.0" | ||
# maybe can be removed when hyper is updated | ||
http = "0.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will take a shot at updating hyper too but in a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the release that caused large incompatibilities was hyper 0.11, couldn't figure out a way to update hyper
ofborg/src/config.rs
Outdated
Credentials::Token(self.github.clone().unwrap().token), | ||
) | ||
.unwrap() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also really like to see less unwraps (even though I introduced most of these in my quick-n-dirty attempt).
9c3a12f
to
5941186
Compare
Reminder @cole-h |
@@ -186,10 +183,11 @@ impl GithubAppVendingMachine { | |||
} | |||
|
|||
fn jwt(&self) -> JWTCredentials { | |||
JWTCredentials::new(self.conf.app_id, self.conf.private_key.clone()) | |||
let key = std::fs::read(self.conf.private_key.clone()).expect("Unable to read private_key"); | |||
JWTCredentials::new(self.conf.app_id, key).expect("Unable to create JWTCredentials") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the upstream expects this to be DER (the raw bytes), whereas we used to use PEM (base64-encoded). Required a small change to the ofborg infra, but otherwise works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's see how this goes.
ran
cargo fmt
onrustfmt 1.5.1
fixed clippy warnings
fixes #595