Support/wasp sw #34
Merged
Support/wasp sw #34
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
joe4dev
commented
Mar 9, 2019
joe4dev
commented
- Add benchmark execution filtering
- Add test cases
- Refactor codebase
- Fix linter warnings
Commit message conventions inspired by: "How to Write a Git Commit Message" Source: https://chris.beams.io/posts/git-commit/
Motivation: 1) The project is dominated by Rails and the Rails docs favor single quote style 2) The majority of existing code follows this style (381 offenses vs 1439) * Rubocop's styleguide: https://github.com/rubocop-hq/ruby-style-guide#strings * Github's styleguide: https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md#strings * Docs: https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/StringLiterals * Reddit discussion: https://www.reddit.com/r/rails/comments/7g7vto/single_or_double_quotes/
Motivation:
1. Prepare project for Ruby 3+
2. Improve performance (minor)
Potential Side Effects:
* Usage of dynamic/inline string modification
* Searched for `<<` appearances:
* One critical appearance: app/models/shell_runner.rb:13
=> Apply `.dup` fix because `"".frozen? #=> true` but `"".dup.frozen? #=> false`
* No regression discovered with test suite.
Background:
* Good explanation in this blogpost series: https://freelancing-gods.com/2017/07/27/an-introduction-to-frozen-string-literals.html
* Rubocopy Docs: https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/FrozenStringLiteralComment
* StackOverflow: https://stackoverflow.com/questions/37799296/what-does-the-comment-frozen-string-literal-true-do
Motivation: git best practice
Rubocop rule: Layout/SpaceAroundOperators
Rubocop rule: Layout/SpaceInsideHashLiteralBraces
Rubocop rules: Layout/SpaceAroundEqualsInParameterDefault
Rubocop rule: Layout/CaseIndentation
Rubocop rule: Layout/IndentationWidth: Use 2 (not 4) spaces for indentation.
Use `key: :value` instead of `:key => :value` Rubocop rule: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
Rubocop rule: Layout/SpaceBeforeBlockBraces: Space missing to the left of {.
Rubocop rule: Layout/IndentationConsistency: Inconsistent indentation detected
Rubocop rule: Style/RedundantReturn: Redundant return detected.
Rubocop rule: Style/Semicolon: Do not use semicolons to terminate expressions.
Rubocop rule: Layout/SpaceInsideParens: Space inside parentheses detected.
Rubocop rule: Layout/SpaceBeforeFirstArg: Put one space between the method name and the first argument.
Rubocop rule: Layout/ElseAlignment: Align else with when.
Rubocop rule: Style/MethodDefParentheses: Use def with parentheses when there are parameters.
Rubocop rule: Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an elsif.
Rubocop rule: Layout/CommentIndentation: Incorrect indentation detected
Rubocop rule: Layout/SpaceInsideBlockBraces: Space missing inside }
Rubocop rule: Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end
Rubocop rule: Layout/LeadingCommentSpace: Missing space after #
Rubocop rule: Style/BracesAroundHashParameters: Redundant curly braces around a hash parameter
Rubocop rule: Layout/SpaceInsideParens: Space inside parentheses detected
Clarify the side effect by adding the `exec` (execution) part to the method name. Fix unclear similarity with `ssh_shell_cmd`
* More flexible and powerful * Handle params interpretation at controller-level instead of passing them (i.e., params[:active]) into models Source: https://www.justinweiss.com/articles/search-and-filter-rails-models-without-bloating-your-controller/
Unable to implement (with reasonable effort).
First attempt:
```ruby
# frozen_string_literal: true
require 'rails_helper'
describe BenchmarkDefinitionsController do
let(:benchmark_definition) { attributes_for(:benchmark_definition) }
# StackOverflow: https://stackoverflow.com/questions/28947241/how-to-post-with-missing-authenticity-token-in-rspec-rails-request-test
context 'invalid authenticity token' do
before do
ActionController::Base.allow_forgery_protection = true
end
it 'should throw an InvalidAuthenticityToken exception' do
# Handle unverified request is triggered first!
expect { post benchmark_definitions_path, params: benchmark_definition }.to raise_error(ActionController::InvalidAuthenticityToken)
end
after do
ActionController::Base.allow_forgery_protection = false
end
end
end
```
This is somewhat related to [Move Statements to Callers](https://refactoring.com/catalog/moveStatementsToCallers.html) combined together with introducing a callback instead of manually calling a method to initialize variables. Notice that this is a special case of a constructor because the actual Ruby-alike constructor (i.e., initialize) is not applicable here due to Rails context. Rails callbacks: https://guides.rubyonrails.org/active_record_callbacks.html
This is somewhat related to [Move Statements to Callers](https://refactoring.com/catalog/moveStatementsToCallers.html) combined together with introducing a callback instead of manually calling a method to initialize variables. Notice that this is a special case of a constructor because the actual Ruby-alike constructor (i.e., initialize) is not applicable here due to Rails context. Rails callbacks: https://guides.rubyonrails.org/active_record_callbacks.html
This is somewhat related to [Move Statements to Callers](https://refactoring.com/catalog/moveStatementsToCallers.html) combined together with introducing a callback instead of manually calling a method to initialize variables. Notice that this is a special case of a constructor because the actual Ruby-alike constructor (i.e., initialize) is not applicable here due to Rails context. Rails callbacks: https://guides.rubyonrails.org/active_record_callbacks.html
See equivalence definition: https://guides.rubyonrails.org/active_record_querying.html#scopes
Pull Request Test Coverage Report for Build 89
💛 - Coveralls |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.