Skip to content

Add Canada Ontario 2018-2022 orthoimagery#2657

Merged
arch0345 merged 3 commits intoosmlab:gh-pagesfrom
galen8183:canada-ontario-ortho
Mar 24, 2025
Merged

Add Canada Ontario 2018-2022 orthoimagery#2657
arch0345 merged 3 commits intoosmlab:gh-pagesfrom
galen8183:canada-ontario-ortho

Conversation

@galen8183
Copy link
Copy Markdown
Contributor

@galen8183 galen8183 commented Mar 19, 2025

Add GEospatial Ontario's (GEO's) 2018-2022 orthophotography WMS service from
https://geohub.lio.gov.on.ca/maps/lio::geospatial-ontario-imagery-data-services/about

Licensed under the OGL Ontario,
a license approved by the OSMF LWG

Obsoletes #2342

Signed-off-by: Galen CC galen8183@gmail.com

@cicku cicku self-assigned this Mar 22, 2025
Signed-off-by: Galen CC <galen8183@gmail.com>
Signed-off-by: Galen CC <galen8183@gmail.com>
@galen8183 galen8183 force-pushed the canada-ontario-ortho branch from 0b8784e to c08190e Compare March 22, 2025 17:51
Also moved all Ontario related layers into its own subfolder
@arch0345
Copy link
Copy Markdown
Contributor

Thanks for your first contribution to ELI! I've gone ahead and specified layers for this endpoint as unfortunately wms_endpoint and wmts sources can't be processed by some editors such as iD/Rapid

@arch0345 arch0345 merged commit 59bedc6 into osmlab:gh-pages Mar 24, 2025
1 check passed
@galen8183
Copy link
Copy Markdown
Contributor Author

Gotcha, I'll keep that in mind for the future :-) thanks!

@galen8183 galen8183 deleted the canada-ontario-ortho branch March 24, 2025 03:39
@cicku
Copy link
Copy Markdown
Collaborator

cicku commented Mar 24, 2025

@arch0345 reverted, I'm not sure why you merged this without my approval.

@galen8183

  1. no need to specify port 443
  2. Is it possible to set license to a LRC URL rather than the osm wiki page

cicku added a commit that referenced this pull request Mar 24, 2025
cicku added a commit that referenced this pull request Mar 24, 2025
@galen8183 galen8183 restored the canada-ontario-ortho branch March 25, 2025 17:12
@galen8183
Copy link
Copy Markdown
Contributor Author

galen8183 commented Mar 25, 2025

  1. Not sure why arch added that GetCapabilities returns the URL with port 443 included, will remove
  2. I modelled this entry off of CityofTorontoImageryLatest, which links to the same OSMF page -- should this instead link to the same licence URL as in the attribution entry, or somewhere else?

@cicku
Copy link
Copy Markdown
Collaborator

cicku commented Mar 30, 2025

We should use the official web page from the hosting/vendor. OSMF page is for clarification, there is no guarantee that the resource (tiles) will have "a license", unless it is clearly mentioned in the official website.

https://geohub.lio.gov.on.ca/maps/lio::geospatial-ontario-imagery-data-services/about has the link to https://www.ontario.ca/page/open-government-licence-ontario, so you should use it.

galen8183 pushed a commit to galen8183/editor-layer-index that referenced this pull request Apr 17, 2025
@andrewharvey
Copy link
Copy Markdown
Collaborator

andrewharvey commented Apr 17, 2025

@arch0345 reverted, I'm not sure why you merged this without my approval.

As far as I know we don't have any documentation or policy for maintainers to follow. We should have something.

  • Can maintainers approve and merge their own PRs? Immediately or only after X time has passed without comment. I'd say yes, but for anything non-trivial preferable to provide some time period, even just 24 hours I think is okay for others to review and provide feedback.
  • How many approvals are needed before a PR can be merged? To reduce maintenance burden and blockages, I'd say no more than 1.
  • Should we have something like code authors, where maintainers more attached or knowledgeable to specific regions can be assigned to review changes within their regions?

I'm not the PR was open for 3 weeks without any comment so after @arch0345 reviewed I think they were okay to merge.

@cicku
Copy link
Copy Markdown
Collaborator

cicku commented Apr 17, 2025

Documentation is one thing, etiquette is something different.

Can maintainers approve and merge their own PRs?

Yes, but highly not recommended. If you have push access why would you still open a PR, because you expect others to review.

How many approvals are needed before a PR can be merged?

Should we have something like code authors

We can.

PR was open for 3 weeks without any comment so after

That's not the case. I assigned myself to review this when the PR was actively modified back and forth, then it was merged suddenly without my review. In fact, the PR has/had issues yet to be addressed. If I assign myself to an active PR then it pretty much indicates that I will work on it.

@arch0345
Copy link
Copy Markdown
Contributor

At least in my experience, there have been several instances where prs that have been supposedly assigned go unreviewed for months. This can be discouraging, especially for new contributors, so after a few days passed I took it upon myself to review this PR. I do acknowledge that I could have asked about what reservations you had in regards to why this shouldn't have been merged, which I will do going forward.

IMO I don't think the issues raised warranted a full on reversion given the points made (port referenced in GetCapabilities, ambiguity of what should be the license_url), but I digress.

@andrewharvey
Copy link
Copy Markdown
Collaborator

That's not the case. I assigned myself to review this when the PR was actively modified back and forth, then it was merged suddenly without my review.

Sorry, you're right, it was merged the day after you self assigned, I thought it was only after a few weeks.

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