Skip to content

Implement full PR review#11

Draft
aneshodza wants to merge 11 commits intomainfrom
feature/implement-full-code-review
Draft

Implement full PR review#11
aneshodza wants to merge 11 commits intomainfrom
feature/implement-full-code-review

Conversation

@aneshodza
Copy link
Contributor

@aneshodza aneshodza commented Jun 20, 2023

Making a todo list so i dont lose oversight:

  • Newlines around headers in readme
  • Remove , nil from ENV.fetch
  • Move CLIENT into init.rb
  • Extract verify_signature into before_action with guard clause
  • Use strong params in webhooks controller
  • This
  • Use Client.compare instead of .commits
  • Delete WebhooksHelper
  • Extract state in pull_request.rb into a method
  • Extract params[:pull_request] in pull_request.rb
  • Change multi line string to partial in render
  • get_ and set_ in hook_listener
  • Use Rails.root.join instead of File.expand_path("#{File.dirname(__FILE__)}
  • This
  • Use i18n everywhere
  • Change time zone to local
  • Add an installation guide to the readme
  • Extend ENV instead of overwriting

@aneshodza aneshodza self-assigned this Jun 20, 2023
@aneshodza
Copy link
Contributor Author

Not sure if strong params make sense, as we have the signatures

@aneshodza
Copy link
Contributor Author

@sihu FYI, this PR is still open. Might make sense to let someone else take this over

@aneshodza aneshodza removed their assignment Aug 23, 2023
@sihu sihu self-assigned this Oct 9, 2023
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.

2 participants