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

Sc 113372 Add Opticks eslint rules to monorepo #79

Merged
merged 16 commits into from
Apr 2, 2024
Merged

Sc 113372 Add Opticks eslint rules to monorepo #79

merged 16 commits into from
Apr 2, 2024

Conversation

Gerrit88
Copy link
Contributor

@Gerrit88 Gerrit88 commented Mar 28, 2024 •
edited
Loading

I have added the eslint-plugin package to the monorepo. The plugin includes rules that check the following:

  • If an experiment has been configured in the passed-in settings
  • If an experiment has concluded based on the passed-in settings
  • If an experiment's result is being assigned to a variable
  • If an experiment has an invalid number of variants

The plan is to publish these changes manually initially, so there is no CI/CD integration.

@Gerrit88 Gerrit88 self-assigned this Mar 28, 2024
@shortcut-integration Shortcut Integration
Copy link

This pull request has been linked to Shortcut Story #113372: Add Opticks eslint rules to mono repo.

Gerrit88
Gerrit88 commented Mar 28, 2024
View reviewed changes
yarn.lock Outdated Show resolved Hide resolved
@Gerrit88 Gerrit88 marked this pull request as ready for review March 28, 2024 14:39
@Gerrit88 Gerrit88 added enhancement New feature or request dependencies Pull requests that update a dependency file labels Mar 28, 2024
jopdeklein
jopdeklein approved these changes Mar 28, 2024
View reviewed changes
Copy link
Collaborator

@jopdeklein jopdeklein left a comment

Choose a reason for hiding this comment

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

I've left a few suggestions, let me know if you want to collaborate on any outstanding tasks I mentioned :)

packages/eslint-plugin/lib/index.js Outdated Show resolved Hide resolved
packages/eslint-plugin/lib/rules/toggle.js Outdated Show resolved Hide resolved
packages/eslint-plugin/lib/rules/toggle.js Outdated Show resolved Hide resolved
packages/eslint-plugin/lib/rules/toggle.js Outdated Show resolved Hide resolved
packages/eslint-plugin/lib/rules/toggle.js Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/opticks.md Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/opticks.md Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/opticks.md Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/opticks.md Outdated Show resolved Hide resolved
packages/eslint-plugin/docs/rules/opticks.md Outdated Show resolved Hide resolved
@Gerrit88 Gerrit88 force-pushed the sc-113372-add-opticks-eslint-rules-to-mono-repo branch from e536a1e to 58d6726 Compare March 28, 2024 16:28
dale-french
dale-french reviewed Mar 28, 2024
View reviewed changes
packages/eslint-plugin/lib/rules/toggle.js Outdated Show resolved Hide resolved
packages/eslint-plugin/lib/rules/toggle.js Outdated Show resolved Hide resolved
packages/eslint-plugin/tests/lib/rules/toggle.js Outdated Show resolved Hide resolved
dale-french
dale-french approved these changes Apr 2, 2024
View reviewed changes
Copy link
Collaborator

@dale-french dale-french left a comment

Choose a reason for hiding this comment

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

Awesome improvement! Thanks @Gerrit88!

@Gerrit88 Gerrit88 merged commit e870dfb into master Apr 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jopdeklein jopdeklein jopdeklein approved these changes

@dale-french dale-french dale-french approved these changes

Assignees

@Gerrit88 Gerrit88

Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Gerrit88 @jopdeklein @dale-french