Skip to content

[Fix #630] do not re-annotate when wrapper matches column_pattern #668

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sashabelozerov
Copy link
Contributor

Fixes #630

@sashabelozerov sashabelozerov force-pushed the fix-wrapper-close-matches-column-pattern branch from 342b5ad to cd21a73 Compare October 5, 2019 10:20
@maxh
Copy link

maxh commented Oct 8, 2019

thanks!

@ctran ctran added this to the v3.1.0 milestone Nov 9, 2019
@ctran
Copy link
Owner

ctran commented Nov 9, 2019

Could you resolve the conflicts? Thanks.

@sashabelozerov sashabelozerov force-pushed the fix-wrapper-close-matches-column-pattern branch from cd21a73 to f42e912 Compare December 15, 2019 09:25
expect(res).to eq(true)

expect(File.read(@model_file_name))
.to eq("# #{wrapper}\n#{@schema_info}# #{wrapper}\n\n#{@file_content}")
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [81/80]

describe 'wrapper value matches column pattern' do
let(:wrapper) { 'Schema generated with `annotate`' }

it 'does not consider this as a new column and does not try to re-annotate' do
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [84/80]

@@ -1685,13 +1685,14 @@ def write_model(file_name, file_content)
def annotate_one_file(options = {})
Annotate.set_defaults(options)
options = Annotate.setup_options(options)
AnnotateModels.annotate_one_file(@model_file_name, @schema_info, :position_in_class, options)
result = AnnotateModels.annotate_one_file(@model_file_name, @schema_info, :position_in_class, options)
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [108/80]

if new_header
new_columns = new_header.scan(column_pattern).sort
new_columns.delete(wrapper_line(options[:wrapper_open])) if options[:wrapper_open]
new_columns.delete(wrapper_line(options[:wrapper_close])) if options[:wrapper_close]
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [92/80]

end
if new_header
new_columns = new_header.scan(column_pattern).sort
new_columns.delete(wrapper_line(options[:wrapper_open])) if options[:wrapper_open]
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [90/80]


if old_header
old_columns = old_header.scan(column_pattern).sort
old_columns.delete(wrapper_line(options[:wrapper_open])) if options[:wrapper_open]
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [90/80]

"# #{wrapper}"
end

def columns_unchanged?(old_content, info_block, options)
Copy link

Choose a reason for hiding this comment

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

Metrics/AbcSize: Assignment Branch Condition size for columns_unchanged? is too high. [26.93/15]
Metrics/CyclomaticComplexity: Cyclomatic complexity for columns_unchanged? is too high. [7/6]
Metrics/MethodLength: Method has too many lines. [17/10]

wrapper_open = options[:wrapper_open] ? "# #{options[:wrapper_open]}\n" : ""
wrapper_close = options[:wrapper_close] ? "# #{options[:wrapper_close]}\n" : ""
wrapper_open = options[:wrapper_open] ? "#{wrapper_line(options[:wrapper_open])}\n" : ""
wrapper_close = options[:wrapper_close] ? "#{wrapper_line(options[:wrapper_close])}\n" : ""
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [97/80]
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


abort "annotate error. #{file_name} needs to be updated, but annotate was run with `--frozen`." if options[:frozen]

# Replace inline the old schema info with the new schema info
wrapper_open = options[:wrapper_open] ? "# #{options[:wrapper_open]}\n" : ""
wrapper_close = options[:wrapper_close] ? "# #{options[:wrapper_close]}\n" : ""
wrapper_open = options[:wrapper_open] ? "#{wrapper_line(options[:wrapper_open])}\n" : ""
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [94/80]
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

new_columns = new_header && new_header.scan(column_pattern).sort

return false if old_columns == new_columns && !options[:force]
return false if columns_unchanged?(old_content, info_block, options) && !options[:force]
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [94/80]

@sashabelozerov sashabelozerov force-pushed the fix-wrapper-close-matches-column-pattern branch 2 times, most recently from 54c3f23 to 7bccaf5 Compare December 15, 2019 09:32
@sashabelozerov
Copy link
Contributor Author

@ctran

1- CI / test (2.3.x) fails on "Setup Ruby" step. I cannot do anything with it from my end, please check.
2- Please explain if I should fix violations reported by Hound bot, because it still passes the check and Rubocop does not detect any offences with the current config.

@drwl
Copy link
Collaborator

drwl commented Dec 18, 2019

@sashabelozerov I fixed CI, if you rebase it should pass in Travis and Github. The Hound bot is new.

@ctran should it be a hard blocker? We already have Rubocop.

@sashabelozerov sashabelozerov force-pushed the fix-wrapper-close-matches-column-pattern branch from 7bccaf5 to 48f5b2b Compare December 18, 2019 06:38
@sashabelozerov
Copy link
Contributor Author

@drwl Thanks, I've rebased the branch.

@ctran ctran removed this from the v3.1.0 milestone Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AnnotateModels needlessly regenerates identical annotations when wrapper_close matches column_pattern
4 participants