-
Notifications
You must be signed in to change notification settings - Fork 29
GBFS Go Implementation Rewrite #177
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
Conversation
…re files with values respecting the minimum allowed timestamp for this field. No logic changes or new functionality introduced.
…ew station information and vehicle status data for typescript
…eplacing with direct JSON unmarshalling
…; add unmarshal example code
- Added new vehicle_status and vehicle_types models based on v3.0. - Updated test fixtures for gbfs.json, gbfs_versions.json, geofencing_zones.json, manifest.json, and system_information.json to reflect changes in versioning and TTL. - Introduced new test fixtures for vehicle availability, vehicle status for carsharing and micromobility, and updated pricing plans. - Removed deprecated system_pricing_plans.json and adjusted vehicle_status.json to align with new structure. - Updated all relevant JSON files to use version "3.1-RC2" and adjusted TTL values accordingly.
…chemas to allow all dateTimes that are iso 8601 comlient
^
(\d{4}(-\d{2}(-\d{2})?)?|\d{8}) # Date: YYYY or YYYY-MM or YYYY-MM-DD or YYYYMMDD
T
(
(\d{2}:\d{2}(:\d{2}(\.\d+)?)?) | # Extended time: hh:mm, hh:mm:ss, hh:mm:ss.sss
(\d{2}:\d{2}(\.\d+)?) | # Extended time: hh:mm.mmm
(\d{2}(\.\d+)?) | # Extended time: hh.hhh
(\d{2}(\d{2}(\d{2}(\.\d+)?)?)?) # Basic time: hhmm, hhmmss, hhmmss.sss
)
(Z|[+-]\d{2}(:?\d{2})?)? # Optional timezone: Z or ±hh:mm or ±hhmm
$
- Changed version from 3.1-RC2 to 3.1-RC across multiple JSON files. - Adjusted TTL values in various files, setting some to 0 and others to specific values. - Updated geofencing zones to set ride_through_allowed to false for specific zones. - Added new files for version 3.1-RC2, including gbfs.json, gbfs_versions.json, geofencing_zones.json, manifest.json, and others. - Introduced new station information files for physical and virtual stations with detailed attributes. - Added vehicle availability and status files for carsharing and micromobility vehicles. - Included comprehensive vehicle types with detailed specifications and assets.
- Deleted obsolete test file for GBFS v3.1-RC1. - Created new test file for GBFS v3.1-RC2 with updated schema paths and test functions. - Updated station_information model to include city field and data structure. - Removed unused fields from system_pricing_plans model. - Updated vehicle_types model to reference v3.0 vehicle_types. - Added new models for GBFS v3.1-RC2 including gbfs, gbfs_versions, geofencing_zones, manifest, station_status, system_alerts, system_information, system_pricing_plans, system_regions, vehicle_availability, vehicle_status, and vehicle_types. - Ensured all new models inherit from their respective v3.1-RC counterparts.
…ions - Removed redundant schema loading and validation functions from gbfsv30_test.go, gbfsv31-RC.go, and gbfsv31-RC2.go. - Replaced calls to specific load and validate functions with a unified approach across all test files. - Improved code readability and maintainability by reducing duplication in test implementations.
|
Hello @DanielKerger, Thank you very much for this significant open source contribution 🙏 cc @davidgamez @emmambd for the next steps. Thanks, |
|
Hi @DanielKerger, Thanks for this contribution! 🚀 This is a large PR with many changes, and since our team has limited capacity at the moment, it may take us some time to review it thoroughly. We want to ensure that we give it the attention it deserves, so we appreciate your patience while we review it. |
|
Hi @davidgamez, Kind regards, Daniel |
Thanks for checking in! We don’t have a specific timeline yet for the review, but it’s still on our radar. As mentioned, it’s a large PR and we want to make sure we review it carefully when we can allocate proper time. We really appreciate your patience and contribution! 🙏 |
|
Hi @DanielKerger, thanks for taking the time to contribute and help improve the GBFS tools. I really like the updated test fixtures and the added support for v3.1-RC2. I also appreciate the way the code is structured — it’s clean and easy to follow. However, I’m a bit concerned about the implications of including future GBFS versions directly in Go and the maintenance effort that might entail. For this PR, did you use any tools or scripts to facilitate your work? That could make upgrading to future versions easier. My main hesitations with this PR are:
Thanks again for your work on this — I’m open to discussing how we could keep the improvements while minimizing upgrade friction. |
|
Hi @Alessandro100, thank you for taking the time to review the PR. Regarding your concerns about maintaining the Go library for future versions, I would offer to maintain the library myself for at least the next two to three years. I could also write a detailed guide on how to maintain the library so that anyone interested can easily pick it up. Regarding your question about the tools used: I used only an IDE with LLM assistance (VSCode with GitHub Copilot). The changes were made manually based on the changes in the JSON schemas. No additional tools were used to generate the code. To address your main hesitations:
Thank you again for your feedback. Please let me know if you have any further questions or concerns. Daniel |
|
First off, thank you so much for your hard work on this PR and the improvements you made, we really appreciate the time and thought you put into it. After discussing with the team, we’ve decided that we won’t be maintaining a manually generated set of GBFS Go language types. While we truly appreciate your offer to maintain and document this work, we need to ensure that key maintenance tasks can be handled by our core team. In our experience, external contributors often have other priorities and commitments that may arise over time, which is completely understandable That said, there are several aspects of your PR we’d like to highlight and build on:
I’d also love to learn more about your experience with the working examples and tests. Feel free to create an issue, come to our contributors meeting or join us on our Slack We also recognize that GBFS version 2.3 is widely used, and adding support for it in the near future would bring great value. As a general guideline for future contributions, we encourage keeping pull requests small and focused whenever possible. One clear goal or fix per PR makes it easier to review, test, and merge. If you’d like to include multiple improvements, consider splitting them into separate PRs for faster feedback and review. Once again, thank you for your contribution and for the insights you’ve shared throughout this PR. Your work has surfaced valuable points that will help guide future improvements. |
This Pull Request introduces a rewrite of the GBFS Go implementation, designed to provide full support for GBFS v2.3 and v3.0, v3.1RC, and v3.1RC2 specifications. The previous codebase has been completely replaced due to incompatibilities with current GBFS standards and insufficient type safety across different Feeds and specification versions.
Key Motivations for the Rewrite:
type Version stringwas declared in all Feeds, leading to redundancy and potential inconsistencies.This rewrite establishes a solid foundation for GBFS processing in Go, with improved type safety, better maintainability, complete v2.3, v3.0, and v3.1RC, v3.1RC2 specification coverage, and a clear path for supporting future specification updates.
My changes include:
Breaking Changes: This is a breaking change that requires updating any existing code that depends on the previous GBFS Go implementation. However, this rewrite now provides GBFS v2.3, v3.1RC, and v3.1RC2 support that was entirely missing before, making it a significant upgrade for production use.