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

Type check jobs #97

Merged
merged 6 commits into from Nov 4, 2023
Merged

Type check jobs #97

merged 6 commits into from Nov 4, 2023

Conversation

tk0miya
Copy link
Contributor

@tk0miya tk0miya commented Nov 1, 2023

Note: This contains #96

@@ -26,6 +26,7 @@ def github_jwt
exp: Time.now.to_i + (10 * 60),
iss: ENV["GITHUB_APP_ID"],
}
JWT.encode(payload, OpenSSL::PKey::RSA.new(ENV["GITHUB_PRIVATE_KEY"].unpack1("m*"), ''), "RS256")
pem = ENV.fetch("GITHUB_PRIVATE_KEY").unpack1("m*") #: String
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed Hash#[] to Hash#fetch because the former one returns String?. Additionally, I added the type comment to indicate the return value of String#unpack is ambigous.

@@ -0,0 +1,3 @@
class ApplicationJob < ActiveJob::Base
include _RbsRailsPathHelpers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this include statement to use path helper methods in the Job subclasses.

@tk0miya tk0miya mentioned this pull request Nov 3, 2023
Copy link
Contributor

@fugakkbn fugakkbn left a comment

Choose a reason for hiding this comment

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

Thanks!

@fugakkbn fugakkbn merged commit 400c356 into kaigionrails:main Nov 4, 2023
2 checks passed
@tk0miya tk0miya deleted the typing_jobs branch November 5, 2023 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants