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.

Sign up for GitHub

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

Jump to bottom

171 isochrone center cannot be set as destination #191

Merged
merged 7 commits into from
Dec 8, 2023
Merged

171 isochrone center cannot be set as destination #191

merged 7 commits into from
Dec 8, 2023

Conversation

merydian
Copy link
Collaborator

@merydian merydian commented Nov 8, 2023

Added the location type option to both isochrone processing algorithms.

@merydian merydian linked an issue Nov 8, 2023 that may be closed by this pull request
Isochrone center cannot be set as "destination" #171
Closed
koebi
koebi requested changes Nov 17, 2023
View reviewed changes
Copy link
Collaborator

@koebi koebi left a comment

Choose a reason for hiding this comment

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

The entry in the changelog and translation is missing.

We could also think about adding an explanation of Location Type in the documentation - do you think that's necessary?

The other option would be to add a tooltip/help/description - QuickOSM does this, via param.setHelp, Saga processing via param.setDescription? Not sure what the best way for this is though…

@merydian
Copy link
Collaborator Author

To me the term Location type is not self explanatory. The documentation does link to the online docs, but i think it wouldn't harm to add another short comment. A comment is probably better than a tooltip i'd say, because everything else is shown as a comment?

@koebi
Copy link
Collaborator

koebi commented Dec 4, 2023

I agree with this, go ahead how you feel it fits.

@merydian
Copy link
Collaborator Author

merydian commented Dec 4, 2023

I called "Location type": "Ortstyp" in the german translation.

@koebi koebi self-assigned this Dec 4, 2023
koebi
koebi reviewed Dec 5, 2023
View reviewed changes
ORStools/help/isochrones_from_layer_de.help Outdated Show resolved Hide resolved
ORStools/help/isochrones_from_point_de.help Outdated Show resolved Hide resolved
@koebi koebi assigned merydian and unassigned koebi Dec 5, 2023
@koebi koebi force-pushed the 171-isochrone-center-cannot-be-set-as-destination branch from 7aa8031 to 00a7bd0 Compare December 8, 2023 11:15
@koebi koebi merged commit f7e4c6a into main Dec 8, 2023
2 checks passed
@koebi koebi deleted the 171-isochrone-center-cannot-be-set-as-destination branch December 8, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koebi koebi koebi approved these changes

Assignees

@merydian merydian

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Isochrone center cannot be set as "destination"
2 participants
@merydian @koebi