-
Notifications
You must be signed in to change notification settings - Fork 244
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
Create safety_data_api.yaml #2733
base: dev
Are you sure you want to change the base?
Conversation
Hi @atalyaalon ,
|
Hi @atalyaalon , |
Hi @atalyaalon |
Hi @atalyaalon |
Hi @atalyaalon ,
|
type: array | ||
items: | ||
type: string | ||
description: List of city names in hebrew (strings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @atalyaalon , this will be changed to int (yishuv_symbol
), right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should. However you and dror should finalize the cities table and queries before implementation since there might be needed some FE changes as well.
schema: | ||
type: array | ||
items: | ||
type: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @atalyaalon , this will be changed to int, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above - I think this should, and that the yishuv and street queries should be integers based both in FE and in BE. However you and dror should finalize it together since there might be needed some FE changes as well, for example he will need streets API per yishuv, as it exists in ANYWAY Media to display name to user, and send the chosen street/s integer in API.
Hi @atalyaalon , @citizen-dror , Regarding the |
|
I added the vehicles calculation in the issue |
You are correct, we want to have all of these fields and not only the 11 listed in the API |
I think so - I think it queries both street1 and street (with OR). But please verify with dror |
You are correct, you can fix it on your request |
I asked Dror regarding only the cities fields used in cities, waiting for his response. |
Once we're done with the discussion, I suggest closing this PR w/o merging and merging only your pr #2735 |
No description provided.