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

chore: additional annotation for notice export #922

Merged
merged 3 commits into from
Jul 8, 2021

Conversation

lionel-nj
Copy link
Contributor

Summary:

This PR provides support to define a new annotation NoticeExport. This annotation is to be used on notice constructor. This specifies the constructor to be considered while exporting notice information.

This relates to #899

Expected behavior:

No code change for now. Same tests should pass.

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
  • [ ] Include screenshot(s) showing how this pull request works and fixes the issue(s)

@lionel-nj lionel-nj requested a review from barbeau July 7, 2021 21:56
@lionel-nj lionel-nj self-assigned this Jul 7, 2021
@lionel-nj lionel-nj linked an issue Jul 7, 2021 that may be closed by this pull request
@lionel-nj lionel-nj changed the title feat: additional annotation for notice export chore: additional annotation for notice export Jul 7, 2021
lionel-nj added 2 commits July 8, 2021 15:45
…-for-notice-export

# Conflicts:
#	core/src/main/java/org/mobilitydata/gtfsvalidator/notice/CsvParsingFailedNotice.java
#	main/src/main/java/org/mobilitydata/gtfsvalidator/validator/BlockTripsWithOverlappingStopTimesValidator.java
#	main/src/main/java/org/mobilitydata/gtfsvalidator/validator/DuplicateRouteNameValidator.java
Copy link
Contributor

@maximearmstrong maximearmstrong left a comment

Choose a reason for hiding this comment

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

LGTM

@maximearmstrong maximearmstrong merged commit ac72323 into master Jul 8, 2021
@maximearmstrong maximearmstrong deleted the task/new-annotation-for-notice-export branch July 8, 2021 21:44
* Annotation to be used on notice constructor. This specifies the constructor to be considered
* while exporting notice information.
*/
public @interface NoticeExport {}
Copy link
Member

Choose a reason for hiding this comment

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

@lionel-nj One comment here - I would suggest renaming this annotation and providing a more detailed description regarding the exact use case for which it exists.

For example, I think SchemaExport may be a better name here, or NoticeSchema, or something with schema in it, to distinguish it from the export of the notices themselves. It's not clear by the name or description of the annotation what exactly the annotation is used for - I'd link to the class that processes it too when that exists.

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.

Ability to dump validator notice schemas to json or yaml
3 participants