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.

Sign up for GitHub

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

Jump to bottom

Refactor AnnotateRoutes.header #714

Merged
merged 2 commits into from
Jan 15, 2020
Merged

Refactor AnnotateRoutes.header #714

merged 2 commits into from
Jan 15, 2020

Conversation

nard-tech
Copy link
Collaborator

I refactored AnnotateRoutes.header for performance, and add AnnotateRoutes.comment to make code DRY.

drwl
drwl approved these changes Jan 15, 2020
View reviewed changes
lib/annotate/annotate_routes.rb

out += ["# #{options[:format_markdown] ? PREFIX_MD : PREFIX}" + (options[:timestamp] ? " (Updated #{Time.now.strftime('%Y-%m-%d %H:%M')})" : '')]
out += ['#']
out << comment(options[:format_markdown] ? PREFIX_MD : PREFIX) + (options[:timestamp] ? " (Updated #{Time.now.strftime('%Y-%m-%d %H:%M')})" : '')
Copy link
Collaborator

Choose a reason for hiding this comment

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

In looking at this line, it seems kinda strange and hard to read.

I'm sure there's a lot of this going on in the code base.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@drwl I agree with you. I want to try to fix it, but I think that it is better to add test cases before refactor this more.

@drwl drwl merged commit da0adbd into ctran:develop Jan 15, 2020
nard-tech added a commit to nard-tech/annotate_models that referenced this pull request Jan 16, 2020
@nard-tech
Merge branch 'feature/refactor_annotate_routes/extract_magic_comments…
ff08701 
…_from_array' into private/develop

* feature/refactor_annotate_routes/extract_magic_comments_from_array:
  Refactor AnnotateRoutes.header (ctran#714)
  Freeze constant AnnotateRoutes::HEADER_ROW (ctran#713)
  Add constants MAGIC_COMMENT_MATCHER (ctran#711)
@nard-tech nard-tech deleted the feature/refactor_annotate_routes/header branch January 16, 2020 18:51
vfonic pushed a commit to vfonic/annotate_models that referenced this pull request May 8, 2020
@nard-tech @vfonic
Refactor AnnotateRoutes.header (ctran#714)
0208fdb 
I refactored `AnnotateRoutes.header` for performance, and add `AnnotateRoutes.comment` to make code DRY.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drwl drwl drwl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@nard-tech @drwl