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

Fix #2095, #1471, #803 - fix fence_type vs material warning #2099

Merged
merged 3 commits into from
Dec 9, 2023

Conversation

Famlam
Copy link
Collaborator

@Famlam Famlam commented Dec 9, 2023

  1. Lower the priority of the Fence with material tag, better use fence_type tag warning (Should "Fence with material tag" really be level 1? #2095)
  2. Make clear that the intention is to add a fence_type, not to remove the material (e.g. fence_type=chain_link + material=metal) (Fence type vs. material #1471, #303210: barrier=fence and material #803)
  3. Convert to mapcss so it can also be loaded into JOSM. (Uses placeholders to avoid people from translating material or fence_type; also drops the tag highway (unrelated) and the tag fix:chair (as from my chair I probably won't be able to distinguish values like barbed_wire from wire; usually it'll require a survey)

1. Lower the priority
2. Make clear that the intention is only to add a `fence_type` tag, e.g. `fence_type=chain_link + material=metal`. The old text implies that `material` should be replaced by `fence_type`, which is incorrect.
@frodrigo frodrigo merged commit d6f5c0a into osm-fr:dev Dec 9, 2023
3 checks passed
@frodrigo
Copy link
Member

frodrigo commented Dec 9, 2023

Nice. Thank you.

@Famlam Famlam deleted the fencetypematerial branch December 9, 2023 19:43
@Famlam
Copy link
Collaborator Author

Famlam commented Dec 17, 2023

@frodrigo Did I do something wrong?
It seems that the subtitle of the issues now contains Fence with material tag, also add fence_type (as expected, this is the new text), but the title is still Fence with material tag, better use fence_type tag (the old title), and the priority is still "high" (instead of low) and the link still refers to TagFix_MultipleTag.py instead of TagFix_MultipleTag2.validator.mapcss.

Example:
https://osmose.openstreetmap.fr/en/issue/5607e6f7-cd01-7fda-9b08-45610d0c4bca

@frodrigo
Copy link
Member

@frodrigo Did I do something wrong? It seems that the subtitle of the issues now contains Fence with material tag, also add fence_type (as expected, this is the new text), but the title is still Fence with material tag, better use fence_type tag (the old title), and the priority is still "high" (instead of low) and the link still refers to TagFix_MultipleTag.py instead of TagFix_MultipleTag2.validator.mapcss.

Example: https://osmose.openstreetmap.fr/en/issue/5607e6f7-cd01-7fda-9b08-45610d0c4bca

It is now OK.

@Famlam
Copy link
Collaborator Author

Famlam commented Dec 22, 2023

Thanks! Only the link is incorrect, but that's because the fix hasn't been merged into master yet

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.

2 participants