Skip to content

Enhance analytics linter to catch non-keyword arguments#7053

Merged
aduth merged 1 commit intomainfrom
aduth-analytics-linter-positional-param
Oct 5, 2022
Merged

Enhance analytics linter to catch non-keyword arguments#7053
aduth merged 1 commit intomainfrom
aduth-analytics-linter-positional-param

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Sep 29, 2022

🛠 Summary of changes

Analytics methods are expected to contain only keyword arguments and an **extra splat, so we should enforce this through the existing lint tooling.

Related example: #7044

📜 Testing Plan

@aduth
Copy link
Contributor Author

aduth commented Sep 29, 2022

Error as expected in GitLab: https://gitlab.login.gov/lg/identity-idp/-/jobs/84884

bundle exec ruby lib/analytics_events_documenter.rb --class-name="IrsAttemptsApi::TrackerEvents" --check --skip-extra-params .yardoc
app/services/irs_attempts_api/tracker_events.rb:393 login_rate_limited unexpected positional parameters ["email"]

changelog: Internal, Analytics Events, Enhance linter to catch non-keyword arguments
@aduth aduth force-pushed the aduth-analytics-linter-positional-param branch from 5339bd9 to 8e18ade Compare September 29, 2022 18:38
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM, good catch!

@aduth aduth merged commit 61b2a4f into main Oct 5, 2022
@aduth aduth deleted the aduth-analytics-linter-positional-param branch October 5, 2022 15:55
jskinne3 pushed a commit that referenced this pull request Oct 12, 2022
changelog: Internal, Analytics Events, Enhance linter to catch non-keyword arguments
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