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

feat: Move key annotation and entity interfaces into a new model module #1362

Merged
merged 2 commits into from
Mar 25, 2023

Conversation

bdferris-v2
Copy link
Collaborator

This will allow us to break up some potential circular dependencies with upcoming support for notice documentation generation, as discussed in #1324 and #1361.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues

…This will allow us to break up some potential circular dependencies with upcoming support for notice documentation generation.
@github-actions
Copy link
Contributor

✅ Rule acceptance tests passed.
New Errors: 0 out of 1423 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1423 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1423 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1423 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1423 sources (~0 %) are corrupted.
Commit: 4a1d7a5
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

@bdferris-v2
Copy link
Collaborator Author

Now wondering if this should have been core/model instead. I could see an argument for splitting up core into other submodules, such as core/io or core/notices.

@davidgamez
Copy link
Member

davidgamez commented Mar 24, 2023

Now wondering if this should have been core/model instead. I could see an argument for splitting up core into other submodules, such as core/io or core/notices.

We can always start with this simple approach(just model) and refactor in the future adding the model back to the core as core/model and the other core children.

@bdferris-v2
Copy link
Collaborator Author

Fair enough.

Copy link
Member

@davidgamez davidgamez left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@github-actions
Copy link
Contributor

✅ Rule acceptance tests passed.
New Errors: 0 out of 1423 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1423 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1423 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1423 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1423 sources (~0 %) are corrupted.
Commit: 0602954
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

@bdferris-v2 bdferris-v2 merged commit 2069c96 into master Mar 25, 2023
@bdferris-v2 bdferris-v2 deleted the issue/1324/model_refactor branch March 25, 2023 00:22
ryon pushed a commit to JarvusInnovations/gtfs-validator that referenced this pull request Apr 1, 2023
…This will allow us to break up some potential circular dependencies with upcoming support for notice documentation generation. (MobilityData#1362)
bradyhunsaker pushed a commit to bradyhunsaker/gtfs-validator that referenced this pull request Apr 25, 2023
…This will allow us to break up some potential circular dependencies with upcoming support for notice documentation generation. (MobilityData#1362)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants