maplibre-gl-js icon indicating copy to clipboard operation
maplibre-gl-js copied to clipboard

Published 20 hours ago • verified publisher icon maplibre

fix or remove specific debug pages

Open xabbu42 opened this issue 3 years ago • 7 comments

Since commit 8a49205cf09a479579142a4d745c932eef49c2b1 a lot of the specialized maps in https://github.com/maplibre/maplibre-gl-js/tree/main/debug no longer work or at least no longer have the original use. For example https://github.com/maplibre/maplibre-gl-js/blob/main/debug/satellite.html is now just the same as index.html instead of a map with satellite imagery. And https://github.com/maplibre/maplibre-gl-js/blob/main/debug/heatmap.html throws an error instead of actually adding the heatmap layer as it expects a particular layer to exist in the map.

For each of the specialized debug pages we should:

  • check if they are useful to us and if they work
  • fix or remove them if necessary

I currently don't have the time to do this but wanted to have an issue as a reminder.

xabbu42 avatar Nov 11 '21 10:11 xabbu42

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar May 11 '22 02:05 github-actions[bot]

I guess this is still an issue, right @xabbu42 ?

wipfli avatar May 12 '22 05:05 wipfli

We probably should organize this folder, maybe even create an index page as part of npm install so that you can easily browse all the different pages. Should be a fun task, I'll keep it in mind :-)

HarelM avatar May 12 '22 06:05 HarelM

still an issue, the example satellite.html still only is a copy of index.html.

xabbu42 avatar May 17 '22 09:05 xabbu42

I've created a way to index all the pages, it should be simple now to open the browser and see which pages are broken and which are working. @xabbu42 any chance you can do it?

HarelM avatar Aug 17 '22 13:08 HarelM

I will try to look at a few, but its usually not just as simple as having a quick check if they work. Some seem to work but no longer cover the same use case like satellite, others don't work but are clearly useful and should be fixed instead of deleted like heatmap.

xabbu42 avatar Aug 23 '22 07:08 xabbu42

I decided to push a solution to #198 first, as some of the debug pages need style specific fixes and we should first use a more complex style and data for our debug pages.

xabbu42 avatar Aug 26 '22 13:08 xabbu42

New path to debug pages for this issue: https://github.com/maplibre/maplibre-gl-js/tree/main/test/debug-pages

austyle avatar Oct 28 '22 14:10 austyle

After reviewing the Debug Pages, my notes and suggestions can be found here: https://github.com/maplibre/maplibre-gl-js/discussions/1823

austyle avatar Nov 07 '22 17:11 austyle

This issue should be higher priority. As a new contributor, it's hard to tell if I broke anything when I don't know what's broken to start with!

rotu avatar Dec 10 '22 15:12 rotu

@rotu feel free to push this forward. This project is community driven. Generally speaking, our CI is what defines if we are good or not, those debug pages are not helpful IMHO and I think they should be removed (at least most of them).

HarelM avatar Dec 10 '22 17:12 HarelM

@HarelM Yes, this project is community driven. And demo pages are a good example of something that discourages my (and probably others') ability to confidently contribute.

Unfortunately, they debug pages are prominently recommended as a way to validate new development. Could you (or another veteran) at least update CONTRIBUTING.md accordingly?

rotu avatar Dec 10 '22 18:12 rotu

The discussion about this specific part of this repo can be found here: https://github.com/maplibre/maplibre-gl-js/discussions/1823 Feel free to share your experience and thought there, I'm sure they are valuable and would give another perspective to the discussion. I would hate to push my opinion before hearing more thought about this topic... Once decided, we can open an issue to track the next steps such as updating the contributing guide etc. I'm sorry to hear this discourage you from contributing, if there's anything I can do to help you in that aspect please let me know. Also please don't take any of my code review change requests personally, I simply want to make sure the quality of this library stays good over time, and sometimes it doesn't pass well when writing. Feel free to ping me via slack if you think something I said was not in place or you need some guidance.

HarelM avatar Dec 10 '22 19:12 HarelM

@HarelM, no worries! You have been very welcoming and I'm not offended at all by this or any code review comments. I hope that I haven't been out of line either!

The things that can help contribute significantly:

  1. Edit the PR template to be actionable.
  • Briefly describe the changes in this PR. (Where? In addition to the changelog entry?)
  • Post benchmark scores. (How? How do I know if the benchmark scores are acceptable?)
  • Manually test the debug page. (Which debug page? How? What am I looking for?)
  1. Resolve benign build warnings and errors. (I have a number of pull requests for this)
  2. Update CONTRIBUTING.md
  • This recommends using the debug page. How, which subpages, and what am I actually looking for?

rotu avatar Dec 10 '22 20:12 rotu

Thanks for the input! I'll see how I can improve the PR template to better explain how to do things, generally speaking, there are good documentation in this repo that explain how to do stuff, I'll see if I can link them from the PR template for better actionable points.

HarelM avatar Dec 10 '22 20:12 HarelM

Seems like this issue can be closed as most of the debug pages were removed.

xabbu42 avatar Apr 18 '23 13:04 xabbu42