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

Support --frozen option for routing annotations #979

Merged
merged 7 commits into from
Mar 30, 2023

Conversation

kg8m
Copy link
Contributor

@kg8m kg8m commented Feb 7, 2023

Hi. Thanks for this great gem! The --frozen option is very useful.

I noticed that the --frozen option doesn't work for routing annotations (rake annotate_routes and annotate --routes). This pull request adds the support for that.

Best regards.

@kg8m kg8m force-pushed the support-routing-frozen branch 2 times, most recently from 1205cb8 to 66ce208 Compare February 12, 2023 09:37
@ctran
Copy link
Owner

ctran commented Mar 29, 2023

@kg8m Would you mind rebase against the latest develop (which has fixed the CI issues)?

Thanks.

@ctran ctran self-assigned this Mar 29, 2023
@ctran ctran added this to the v3.2.1 milestone Mar 29, 2023
Signed-off-by: kg8m <takumi.kagiyama@gmail.com>
@ctran
Copy link
Owner

ctran commented Mar 29, 2023

Could I convince you to add an unit test/spec for this new option?

Signed-off-by: kg8m <takumi.kagiyama@gmail.com>
@kg8m kg8m requested a review from ctran March 30, 2023 16:39
Signed-off-by: kg8m <takumi.kagiyama@gmail.com>
@kg8m
Copy link
Contributor Author

kg8m commented Mar 30, 2023

@ctran
I fixed the RuboCop error and added tests for the --frozen option of routing annotations. Could you please review the changes?

Best regards.

lib/annotate/annotate_routes.rb Show resolved Hide resolved
spec/lib/annotate/annotate_routes_spec.rb Outdated Show resolved Hide resolved
Copy link
Owner

@ctran ctran left a comment

Choose a reason for hiding this comment

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

I made the suggestions directly on your fork, hope you don't mind.

@ctran ctran merged commit 10a7a76 into ctran:develop Mar 30, 2023
3 checks passed
@kg8m
Copy link
Contributor Author

kg8m commented Mar 31, 2023

@ctran
Thank you so much!

matteolc pushed a commit to matteolc/annotate_models that referenced this pull request Mar 31, 2023
commit 22ab676
Author: Cuong Tran <ctran@users.noreply.github.com>
Date:   Thu Mar 30 15:17:37 2023 -0700

    chore: remove broken badges from README.md

commit 10a7a76
Author: Takumi KAGIYAMA <694547+kg8m@users.noreply.github.com>
Date:   Fri Mar 31 07:15:07 2023 +0900

    Support `--frozen` option for routing annotations (ctran#979)

    The `--frozen` option previously deal only model annotations.  This change will support route annotations as well.

    ---------

    Signed-off-by: kg8m <takumi.kagiyama@gmail.com>
    Co-authored-by: Cuong Tran <ctran@users.noreply.github.com>

commit a28fef3
Author: Jim Jowdy <jjowdy@gmail.com>
Date:   Wed Mar 29 15:09:11 2023 -0700

    Fix retrieve_indexes_from_table when indexes is empty and base table does not exist. (ctran#849)

    Some tables may have a table_name_prefix but no indexes. Previous versions of
    the code would strip the prefix and look for indexes on the resulting table
    which likely would not exist. This causes DB errors, at least in MySQL. So now
    check if the new table exists first before trying to show its indexes.

commit 13b532d
Author: Cuong Tran <ctran@users.noreply.github.com>
Date:   Wed Mar 29 01:51:15 2023 -0700

    Update codeql-analysis.yml

commit ea4cd00
Author: Lovro Bikić <lovro.bikic@infinum.hr>
Date:   Wed Mar 29 10:31:01 2023 +0200

    Add support for annotating check constraints (ctran#868)

    This adds annotation of check constraints with an option to disable/enable annotation. Most of the work done in this PR is based off of existing implementation for annotating indexes and foreign keys.

    Signed-off-by: Lovro Bikic <lovro.bikic@infinum.hr>

commit 76a1804
Author: Lovro Bikić <lovro.bikic@gmail.com>
Date:   Wed Mar 29 10:18:24 2023 +0200

    Fix flaky specs (ctran#980)

    Signed-off-by: Lovro Bikic <lovro.bikic@infinum.hr>
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.

None yet

2 participants