Skip to content

Conversation

@alexevanczuk
Copy link
Contributor

What are you trying to accomplish?

Follow-up to #276 (comment)

In this PR, we're moving test files into folder structures that matches their naming convention. Although we are not using zeitwerk to load test files, we thought this is a best practice that improves navigatability of the codebase.

What approach did you choose and why?

I chose test/* as a sort of "autoload" path in zeitwerk parlance, with anything that follows that to constitute a portion of the constant namespacing.

That is, test/support and test/integration are organizational units to organize test files into general buckets. Everything that follows represents a portion of the namespace.

What should reviewers focus on?

Does the nesting make sense?
Should we enforce this by actually loading test constants with zeitwerk?

Type of Change

  • Bugfix
  • New feature
  • Non-breaking change (a change that doesn't alter functionality - i.e., code refactor, configs, etc.)

Checklist

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • It is safe to rollback this change.

@alexevanczuk alexevanczuk requested a review from a team as a code owner December 15, 2022 20:24
Copy link
Member

@gmcgibbon gmcgibbon left a comment

Choose a reason for hiding this comment

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

Thanks!

@alexevanczuk alexevanczuk merged commit 8476bb3 into main Dec 15, 2022
@alexevanczuk alexevanczuk deleted the ae-move-test-files branch December 15, 2022 21:22
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems March 1, 2023 19:58 Inactive
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