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

Add constants MAGIC_COMMENT_MATCHER #711

Merged

Conversation

nard-tech
Copy link
Collaborator

I added AnnotateModels::MAGIC_COMMENT_MATCHER and AnnotateRoutes::MAGIC_COMMENT_MATCHER instad of AnnotateModels.magic_comment_matcher and AnnotateRoutes.magic_comment_matcher.

I think that they should be constants because the result of Regexp is always fixed.

@nard-tech nard-tech force-pushed the feature/add-constants-magic-comment-matcher branch from e11aaac to 59f11d9 Compare January 7, 2020 17:06
@nard-tech
Copy link
Collaborator Author

nard-tech commented Jan 7, 2020

The Regexp in AnnotateModels and that in AnnotateRoutes are different. Is it OK?

I think that AnnotateModels::MAGIC_COMMENT_MATCHER and AnnotateRoutes::MAGIC_COMMENT_MATCHER may be unified to another constant (e.g. Annotate::MAGIC_COMMENT_MATCHER).

@drwl
Copy link
Collaborator

drwl commented Jan 15, 2020

The Regexp in AnnotateModels and that in AnnotateRoutes are different. Is it OK?

I think that AnnotateModels::MAGIC_COMMENT_MATCHER and AnnotateRoutes::MAGIC_COMMENT_MATCHER may be unified to another constant (e.g. Annotate::MAGIC_COMMENT_MATCHER).

@nard-tech It should be okay that they're different for the time being. I think without better test coverage we shouldn't make behavior changes.

@drwl drwl merged commit 2213ece into ctran:develop Jan 15, 2020
@nard-tech
Copy link
Collaborator Author

@drwl

I think without better test coverage we shouldn't make behavior changes.

All right. Thank you for your reply.

@nard-tech nard-tech deleted the feature/add-constants-magic-comment-matcher branch January 15, 2020 21:42
nard-tech added a commit to nard-tech/annotate_models that referenced this pull request Jan 16, 2020
…_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)
vfonic pushed a commit to vfonic/annotate_models that referenced this pull request May 8, 2020
I added `AnnotateModels::MAGIC_COMMENT_MATCHER` and `AnnotateRoutes::MAGIC_COMMENT_MATCHER` instad of `AnnotateModels.magic_comment_matcher` and `AnnotateRoutes.magic_comment_matcher`.

I think that they should be constants because the result of Regexp is always fixed.
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