Skip to content

Fixed Required/Conditionally Required/Optional coding errors - #4

Closed
giocorti wants to merge 10 commits into
masterfrom
coding-errors
Closed

Fixed Required/Conditionally Required/Optional coding errors#4
giocorti wants to merge 10 commits into
masterfrom
coding-errors

Conversation

@giocorti

@giocorti giocorti commented Oct 22, 2018

Copy link
Copy Markdown

The following fields have been recoded as Conditionally Required:
agency.txt: agency_id
stops.txt: zone_id
routes.txt: agency_id
fare_attributes.txt: agency_id
stop_times.txt: arrival_time and departure_time

@antrim

antrim commented Oct 22, 2018

Copy link
Copy Markdown

@giocorti : Small thing, but can you remove the .DS_Store and any other invisible Mac OS files from the branch/PR?

LGTM except for the .DS_Store.

To confirm: The only change is the change of 4 fields from "Optional" to "Conditionally required". Can you list those fields in the pull request text?

@LeoFrachet

Copy link
Copy Markdown

@giocorti: You should add .* to your .gitignore.

@LeoFrachet

Copy link
Copy Markdown

@giocorti: Updating the gitignore works for the future, but do not remove the existing files. Sorry if my comment was misleading. You still have to remove the one you've already committed by hand.

@barbeau

barbeau commented Oct 23, 2018

Copy link
Copy Markdown
Member

IMHO, before a PR is opened on the main google/transit repo the .gitignore should be removed (along with any stray files).

Comment thread gtfs/spec/en/reference.md Outdated
| Field Name | Type | Required | Details |
| ------ | ------ | ------ | ------ |
| agency_id | ID | Optional | The **agency_id** field contains an ID that uniquely identifies a transit agency. A transit feed may represent data from more than one agency. This field is optional for transit feeds that only contain data for a single agency. |
| agency_id | ID | **Conditionally required** | The **agency_id** field contains an ID that uniquely identifies a transit agency. A transit feed may represent data from more than one agency. This field is optional for transit feeds that only contain data for a single agency. |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For each of these fields, I suggest we follow the following format in the Details field at the end of the description:

This field is required if..., otherwise it is optional.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've added the suggested language. Let me know what you think.

Specify when field is required
Comment thread gtfs/spec/en/reference.md Outdated
| ------ | ------ | ------ | ------ |
| route_id | ID | **Required** | The **route_id** field contains an ID that uniquely identifies a route. |
| agency_id | ID | Optional | The **agency_id** field defines an agency for the specified route. This value is referenced from the [agency.txt](#agencytxt) file. Use this field when you are providing data for routes from more than one agency. |
| agency_id | ID | **Conditionally required** | The **agency_id** field defines an agency for the specified route. This value is referenced from the [agency.txt](#agencytxt) file. This field is required for feeds that contain more than one transit agency, otherwise it is optional. |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd suggest:

is required for feeds that contain more than one transit agency in agency.txt

Comment thread gtfs/spec/en/reference.md Outdated
| | | | * **2** - Passenger may transfer twice. |
| | | | * **(empty)** - If this field is empty, unlimited transfers are permitted. |
| agency_id | ID | Optional | Required for feeds with multiple agencies defined in the agency.txt file. Each fare attribute must specify an agency_id value to indicate which agency the fare applies to. |
| agency_id | ID | **Conditionally required** | Required for feeds with multiple agencies defined in the agency.txt file. Each fare attribute must specify an agency_id value to indicate which agency the fare applies to. This field is required for feeds that contain more than one transit agency, otherwise it is optional.|

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same - add in agency.txt

@barbeau barbeau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@giocorti Two more small changes in-line. And .gitignore is still included. Other than that LGTM.

@LeoFrachet

Copy link
Copy Markdown

LGTM

@barbeau

barbeau commented Oct 24, 2018

Copy link
Copy Markdown
Member

LGTM, thanks @giocorti!

@antrim antrim left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@giocorti giocorti closed this Mar 12, 2019
skalexch added a commit that referenced this pull request Jul 13, 2026
# This is the 1st commit message:

add tripupdate clarification

# This is the commit message #2:

fixed formatting

# This is the commit message #3:

Clarify detour description in service alerts

Updated detour description for clarity.
# This is the commit message #4:

Clarify DETOUR effect description in reference.md

Updated the description of the DETOUR effect to clarify that it refers to stops not being served.
# This is the commit message #5:

better detour definition
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.

4 participants