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

Location Groups July 2024 #1017

Merged
merged 25 commits into from
Feb 19, 2025

Conversation

miles-grant-ibigroup
Copy link
Contributor

@miles-grant-ibigroup miles-grant-ibigroup commented Jul 11, 2024

Location Group support following latest flex spec. Works with data tools-server dev-flex branch

@miles-grant-ibigroup miles-grant-ibigroup self-assigned this Jul 11, 2024
@miles-grant-ibigroup miles-grant-ibigroup changed the base branch from dev to dev-flex-2024 July 11, 2024 18:43
@miles-grant-ibigroup miles-grant-ibigroup added WIP BLOCKED Blocked (waiting on another PR to be merged) Flex labels Jul 11, 2024
@clinephi
Copy link

Excited for this update!

@ibi-group ibi-group locked as too heated and limited conversation to collaborators Sep 12, 2024
@miles-grant-ibigroup miles-grant-ibigroup marked this pull request as ready for review September 17, 2024 14:36
@miles-grant-ibigroup miles-grant-ibigroup removed the BLOCKED Blocked (waiting on another PR to be merged) label Dec 16, 2024
Copy link
Contributor

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

First round of comments.

@miles-grant-ibigroup miles-grant-ibigroup self-assigned this Feb 3, 2025
Copy link
Contributor

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Next round of comments...

Copy link
Contributor

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Nice cleaning up the interim flex stuff and improving the types. I am going to give the benefit of doubt regarding the flow stuff. There are few nits but nothing fundamentally blocking.

@@ -863,6 +824,13 @@ export type GtfsLocation = {|
zone_id?: string
|}

// export type GtfsLocationGroup = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll need it to show location groups on the map.

@binh-dam-ibigroup binh-dam-ibigroup removed their assignment Feb 10, 2025
Copy link

@josh-willis-arcadis josh-willis-arcadis left a comment

Choose a reason for hiding this comment

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

Nothing major from me. Code looks good.

@@ -180,6 +182,7 @@ export default class EntityDetails extends Component<Props, State> {
</Form>
</div>
)

Choose a reason for hiding this comment

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

nit. remove extra space.

@miles-grant-ibigroup miles-grant-ibigroup merged commit 535730c into dev-flex-2024 Feb 19, 2025
2 checks passed
@miles-grant-ibigroup miles-grant-ibigroup deleted the pattern-location-groups-july-2024 branch February 19, 2025 20:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants