search

gravitystorm / openstreetmap-carto

A general-purpose OpenStreetMap mapnik style, in CartoCSS
Other
1.54k stars 823 forks source link

Review onboarding #2291

Open pnorman opened 8 years ago

pnorman commented 8 years ago

We haven't really looked at this in a long time, and all of the maintainers are experienced enough that getting started is removed from what we do that we're not the best people to consider it. We should review our onboarding.

matthijsmelissen commented 8 years ago

Thanks for opening this issue, I'm very curious for the response from new or potential contributors.

don-vip commented 8 years ago

Something strange at first glance is the ownership of this repository. If you want maximal visibility as it is now, for a long time, the de-facto standard OSM style, shouldn't it be transferred to @openstreetmap organization?

pnorman commented 8 years ago

Something strange at first glance is the ownership of this repository. If you want maximal visibility as it is now, for a long time, the de-facto standard OSM style, shouldn't it be transferred to @openstreetmap organization?

I've preferred not having it on the @openstreetmap organization. Part of this is because I don't know what being on that organization would entail, part of it is I view this stylesheet as the one chosen to be on tile.osm.org not the stylesheet for osm.org, and part of it is that it's fairly common for OSM software projects not to be on the openstreetmap organization. Examples include JOSM, taginfo, cgimap, and overpass.

The traffic information from GitHub is maintainer only, but we're getting >50 unique cloners and >1k UVs a week and plenty of activity on the issue tracker. I think it's an issue of converting that to contributors.

CC @geostonemarten and @Ircama as people who have recently gotten started

nebulon42 commented 8 years ago

Maybe a tutorial document for new developers might be useful. https://gist.github.com/nebulon42/e5f73e511d44726ef66c7e13eb9e3eca contains an initial version. Comments and additions are welcome. Since Gists do not allow PRs from others I could set up a branch for inclusion in osm-carto if you think that would be good.

Ircama commented 8 years ago

Incentivizing the involvement of new contributors is always an essential process for providing quality, progress, innovation and long life to an open project and any initiative towards this target I think is appropriate.

Guidelines, recommendations and documented best practices would be important to save time and concentrate all efforts on the quality of implementation, on the related testing and QA.

BTW on the installation I tried to provide something here by now: https://github.com/Ircama/switch2osm.github.io/blob/tilemill-osm-carto/tilemill-osm-carto.md Hints related to the environment to use locally or in cloud would also be useful.

Finally, I’m afraid that this issue unfortunately references an early PR of mine as an example to blame. Indeed I did not realize of some problems there and anyway did not mean to procure any irritation.

pnorman commented 8 years ago

Finally, I’m afraid that this issue unfortunately references an early PR of mine as an example to blame. Indeed I did not realize of some problems there and anyway did not mean to procure any irritation.

One of us should have stepped in and said something. Knowing what to do here relies on unwritten knowledge.

Ircama commented 8 years ago

It is generally useful to write down best practices as well as advise for improvements when a problem arises (maybe pointing to the best practice document). This especially for contributors who may join the group just for a specific purpose (and mine is to verify the feasibility to improve the rendering of mountain areas described at http://wiki.openstreetmap.org/wiki/Hiking, avoiding impacts for the current adoptions, which I think would provide great benefit to OSM).

dieterdreist commented 8 years ago

2016-08-26 12:19 GMT+02:00 Paul Norman notifications@github.com:

part of it is I view this stylesheet as the one chosen to be on tile.osm.org not the stylesheet for osm.org,

actually this is the style sheet that was developed for osm.org, for many years, there's a straight continuity from the former mapnik stylesheet to osm-carto: when Andy ported the xml style to carto he explicitly tried to make it as similar as possible. On this background, this style sheet is different to the other style sheets chosen to be on the front page, but not rendered with osmf controlled ressources and (AFAIK) not developed for the main osm.org page.

and part of it is that it's fairly common for OSM software projects not to be on the openstreetmap organization. Examples include JOSM, taginfo, cgimap, and overpass.

none of them is default on the front page and JOSM and cgimap are at least linked/mirrored from the official repo: https://github.com/openstreetmap?page=1 Maybe osm-carto and taginfo could also be included this way to the git repo.

Cheers, Martin

imagico commented 8 years ago

One problem with getting started is that initially you often run into problems but do not realize or misinterpret the nature of these problems due to lack of background knowledge.

A particular problem that should be communicated is the biting off more than you can chew issue. Everyone runs into this on occasion. You think about making just a small change but realize that this has a tons of side effects that require significant work to actually result in an overall improvement. There are three ways to deal with this

Usually newcomers do not make this choice deliberately. IMO one of the key tasks of the maintainers is to help contributors choosing the second or third approach. The desire to be friendly to newcomers will often put pressure on accepting the first option though - which then frequently leads to one problem solved but two or three additional ones created.

Regarding cartographic review - guidance from the maintainers is central to contributors knowing where they stand. A laisser-faire-approach here is not generally helpful. New contributors often bring in interesting new ideas and it is crucial to look at these with an open mind but it is also good IMO to be upfront about the fact that choices are ultimately subjective. It is important where possible to explain your reasoning for certain choices but ultimately you cannot prove something does not work design wise.

Looking back at my first contributions - one of the positive experiences was that i was able to change negative opinions on certain changes through arguments. This is both good in the way that arguments can make a difference and because bringing forward convincing arguments for your change is demanded from contributors.

pnorman commented 8 years ago

actually this is the style sheet that was developed for osm.org,

I'd rather keep discussion on what this stylesheet is elsewhere - it's minimally connected with onboarding. For onboarding we already have someone interested in the style and some interest in contributing, not just commenting on issues.

Most new contributors seem to get started with adding features rather than improving cartography or other more valuable types of contributions. Is this because that's what new people are interested in, or are we filtering out those with other interests implicitly with our documentation.

matthijsmelissen commented 8 years ago

I can also imagine adding new features is one of the more easy things to do.

kocio-pl commented 8 years ago

I'm not sure if this is true in general. I started with adding shop icons and that was not that easy (although we have many examples to follow them), while changing some colors lately was dead easy from a technical point of view.

Maybe it's not about what is easy and people just care more for their favorite missing features than for optimizing what is already visible on the map? Some people have simple comments on polish forum about what could be changed (like making tertiary yellow or that cemetery in the forest should be more visible), but most of the time they don't even try to create a ticket and we don't see them here - and I don't understand why.

geostonemarten commented 8 years ago

but most of the time they don't even try to create a ticket and we don't see them here - and I don't understand why.

It is simple. You need speak in english first. You need know that project. There is not bypass to signal a bug on OSM.org or other site using this stylesheet... Users or Openstreetmap contributors have a preference for their local community. If you want to add more connection it is necessary to add a refering contributor. For exemple on French, German, English. Next it's necessary to add a consensus on deferents choices (style).

There is 3 approachs for a contributor : Add missing elements, fix elements, optimise project.

For a contributor of this project you need know developpement.

note : TileMill is no longer in active development. For our most up-to-date map design tool, check out Mapbox Studio.

Ircama commented 8 years ago

TileMill is no longer in active development. For our most up-to-date map design tool, check out Mapbox Studio.

Even if the installation of Mapbox Studio on Windows is straightforward, I could not succeed in configuring it to run openstreetmap-carto with the PostgreSQL instance already supporting TileMill (neither project.mml nor project.yaml renamed to project.yml appear to be compatible). Is there a detailed (and tested) step-by-step guide on this specific configuration?

matthijsmelissen commented 8 years ago

I don't think many of us run Windows.

Do you get any specific error messages?

Ircama commented 8 years ago

These are the steps I do:

Error Invalid tilesource protocol: nul

I am not sure these are the correct steps. There should be a problem in project.yml (project.yaml of openstreetmap-carto) because the default (much simpler) projects created by Mapbox through default presets work.

pnorman commented 8 years ago

I don't think many of us run Windows.

I do, but I run kosmtik on a linux machine and access it over the network.

Even if the installation of Mapbox Studio on Windows is straightforward, I could not succeed in configuring it to run openstreetmap-carto with the PostgreSQL instance already supporting TileMill (neither project.mml nor project.yaml renamed to project.yml appear to be compatible). Is there a detailed (and tested) step-by-step guide on this specific configuration?

Mapbox Studio Classic is for developing styles which use a different technology and will not work with OpenStreetMap Carto.

Ircama commented 8 years ago

I do, but I run kosmtik on a linux machine and access it over the network.

That is the way I also had to do at the end.

Mapbox Studio Classic is for developing styles which use a different technology and will not work with OpenStreetMap Carto.

Thanks a lot for this clarification. I also imagined that Mapbox Studio Classic was exploiting different formats (and Mapbox Studio should implement a vector based rendering engine, so not Mapnik).

matkoniecz commented 7 years ago

https://switch2osm.org/loading-osm-data/ that I used when I first started developing for this style and wanted to reuse now appears to be outdated. It may be fixable by https://github.com/switch2osm/switch2osm.github.io/issues/34

Ircama commented 7 years ago

I have collected some installation details here and occasionally I try to keep it updated by checking openstreetmap-carto documentation and recommendations (unfortunately, I cannot find time enough to answer requests of support arising there). It should anyway provide more recent and up to date information than https://switch2osm.org.

kocio-pl commented 7 years ago

I have recently expanded Wiki page about Standard tile layer (which is mostly about osm-carto style) and included a link to your great tutorials. We can expand this page even more and since it's not the official documentation, there can be more useful informations which are outside the scope of the project itself.

kocio-pl commented 6 years ago

This could be helpful for onboarding: #2256.

kocio-pl commented 6 years ago

I believe we're now on the right track for onboarding. We have tools (like Docker installation framework), wiki documentation explaining basic facts about the style, templates for issue and PR tickets, and we have people starting and willing to design things, discuss them and produce a code with testing. The last thing needs some more work, but I'm pretty happy with more people able to make the code after using just few links.

I hope we can make it sustainable trend. The core actions that I think made the change are:

I might be wrong and the real reasons might be different, but at least it flies now.

matthijsmelissen commented 6 years ago

Thanks @kocio-pl for your great contributions in this respect!

kocio-pl commented 6 years ago

Since at the moment onboarding looks quite healthy, I will close the ticket. Might be reopened if there will be a need to do it again.

kocio-pl commented 5 years ago

A year later the situation is different and the onboarding new participants is where we should make much better. Big PRs might be still a problem (also to me), but my experience shows that better communication and actively supporting new participants propositions by people with merging rights is more vital than any technical obstacles we have - at least it worked good last year despite all of them.

I think this is important and general problem, not to be solved by any single action or review, rather to keep an eye on it until we have again raising activity of some new contributors.

jeisenbe commented 5 years ago

Maintainers: could you lable some more issues as "good first issue"? I was going to link to this label from #3782, but there is only one issue currently listed.

imagico commented 5 years ago

I labeled a few issues yesterday.

As mentioned previously current issue reporting has quite a strong preference on feature addition requests and more complex this styling does not work so well in certain situations issues. This is however not because there are no simple issues that would be a good choice as a first issue to fix. So identifying specific and well compartmentalized problems that might not be so serious but still could use fixing is a task that could use work.

Of course we also have a lot of issues that are clear and have relatively simple solutions but which are not suited as a first issue because the solution is controversial. Many feature addition requests belong to this category of course. Bringing such issues to a decision by either identifying a consensus solution or agreement to close as wontfix/declined would be very useful work.

If people who are interested in maps and map style development but can't or don't want to actively work with the code these are two fields where help would be much appreciated.

jeisenbe commented 5 years ago

these are two fields where help would be much appreciated:

1) "identifying specific and well compartmentalized problems that might not be so serious but still could use fixing"

2)? "Bringing issues to a decision by either identifying a consensus solution or agreement to close as wontfix/declined would be very useful work"

I can see how 1) is useful and might be appropriate for new contributors, but now should a new or infrequent contributor help with 2)? Or is 2) a suggestion for the maintainers, who are able to mark an issue as wontfix/declined?

imagico commented 5 years ago

I considered neither of these necessarily to be a task for a newbie developer. But there are a lot of people who are interested in this style and possibly want to contribute but who don't think actual style development is for them. Depending on qualifications, talent and interest these two fields might be suitable for those. (1) requires a keen eye for cartography but does not depend much on understanding the inner workings of the style and (2) requires good social skills but also not necessarily does much depend on in depth technical understanding. Since controversial issues are often ultimately a conflict between maintainers an outside perspective can help there.

jeisenbe commented 5 years ago

The most difficult thing as a new contributor was figuring out how to make changes to project.mml without breaking things. It's helpful that all of the amenity layers are now combined. But I'm still learning about the SQL queries used there, and honestly I mainly work it out by copying examples in other parts of the code, or in other map styles.

Is there a good link to relevant resources about SQL? Should we write a basic description of how the layers in project.mml work, and how the different queries function, or has this already been done elsewhere?

kocio-pl commented 5 years ago

@jeisenbe Please, consider proposing smaller changes in general. I also think that putting a lot of images makes it even harder for people to join the discussion. I'm glad that you put so much effort and find illustrations, but PR is a tool for deciding and so much data makes it hard to consider (or even read and view...) the most important and relevant things. It is already hard for me and I believe it sets even higher bar for joining other people.

It all makes OSM Carto less of a community project, which might be OK for some time (especially with so many ideas and decisive power you have), but in the longer run I think we need to take care of attracting different community members to get involved. It is long term work and is in the domain of soft actions, but in the end I believe it is important for the project.

I don't think data overload in PRs is the only reason, but I see it as a problem that typically only you and Christoph are ready to talk about all the details at once. This also means smaller chances to get wider feedback (however this is a bit off-topic for this ticket). It is hard for me already, so I try to make a short notices to make discussion more concise, but unfortunately I see no possibility to stick to the core and feel that the problem is well reviewed. I believe for novice users it is much worse.

It would be good to find also some other ways to attract more people to collaborate here, I just wanted to start with something obvious. Please don't take it as a personal punch, I'm happy with you taking the leadership, I just would like to see more people collaborating.

imagico commented 5 years ago

I concur on the scope of images in a PR. If a change requires a larger number of illustrations to be properly evaluated that should be separated from the PR. Not sure if github provides good options for this. @matkoniecz for his road work back many years ago used diary entries to show a larger number of rendering samples but there certainly are other options.

As maintainers we should try to set a good example how to work on changes in a way that invites productive cooperation, specifically a PR should be designed in a way so that useful and productive comments and reviews are possible without investing too much time and energy.

As a practical suggestion: If you see that your approach to a problem changes significantly after you have started a PR and a significant part of your illustrations and explanations of your changes becomes obsolete make generous use of the possibility to close the old one and open a new PR with your new approach. This makes it easier for people to look at and comment on your current state of work without having to look through and filter outdated information.

Personally unless there is specific delicate color relationships and other contextual design aspects to consider i usually don't need a large number of specific image examples to assess a change. The most important thing is usually to have all the rules implemented by a change shown once so you can see what it actually does, not necessarily show the same thing in a lot of different settings.

When in https://github.com/gravitystorm/openstreetmap-carto/pull/3856#issuecomment-526864741 i pointed out the lack of meaningful examples for the need of the change that might have been interpreted as a call to show more sample renderings. It was however only meant as a call for links to some places where the current rendering is bad and the change would lead to improvements. With a change like this i can usually reasonably well imagine how things will look like in different settings without the need to see this in actual rendering.

Also as pointed out in https://github.com/gravitystorm/openstreetmap-carto/pull/3856#issuecomment-529097282 when you realize more work is required or desirable on your PR before merging marking it as a WIP is useful. This way people can consciously decide if they want to contribute and comment on your work in real time or just want to look at the result when finished. You could even go a step further and once you have finished work let it rest for 1-2 days before you ask for reviews. Because at least for me waiting some time to let a matter settle in my mind sometimes makes me realize things i have not seen while working on it.

As a general rule of thumb for everyone creating a PR here: You should show in examples what visual changes you have made but it is usually neither necessary nor necessarily desirable (at least not inline in the PR) to show more examples than are required to show exactly that. The examples are not meant to be sufficient for the maintainers to completely assess the impact of the change on the map in any kind of situation without testing it and looking at the code and considering how it will practically work if used.

kocio-pl commented 5 years ago

I agree with these remarks.

What I also think would help is making explicit what are the blockers for commenting people (but also a short list of what is good or undecided or not that important can work), because that's the most important information for potential code merging decision. This way we don't have to negotiate every detail (from years of experience here - it's not working, no matter how hard we try) and concentrate on what is main problem. I believe that GitHub "change request" review is the best tool for making it clear.

I also think making messages shorter helps to make decision easier. It's not to say that lengthy discussions are bad, but that keeps good code (the one we agree on) away from merging.

jeisenbe commented 5 years ago

I've marked some more issues with the labels "good first issue", and also the new label "help wanted" for issues that are a little more difficult but which would be good to solve.

There is a new github feature which will suggest issues with either of these labels for new contributors.

It might also be helpful to close some issues which should not be recommended.

kocio-pl commented 5 years ago

Good to hear that. It's a constant job to onboard new members and it's a part of different domains (team building, politics/power management, communication with OSM community etc.). However from my experience the most important step is to invite people whenever possible. I started doing it from a sheer frustration when I was doing a lot and people asked new things in tickets, but it proved to be surprisingly effective.

Of course clean code with a lot of comments, easy to set up Docker environment, ticket labels etc. are also important, but no amount of preparations can replace asking people directly and delegating work. There is a book I recommend reading ("Producing Open Source Software. How to Run a Successful Free Software Project" by Karl Fogel) and this chapter is especially about volunteers management:

https://producingoss.com/en/managing-participants.html#delegation

@jeisenbe Since you are one of the new team members, what did help you to get on board and what was hard? Please share your experience if possible.

Of course anyone else is also welcome to share her own experience and name the obstacles - feel free to speak up, no matter how involved in OSM Carto you are. It's important to see the project from the novice point of view.

kocio-pl commented 4 years ago

@jeisenbe Since you are one of the new team members, what did help you to get on board and what was hard? Please share your experience if possible.

I'd like to hear from you to help onboarding more people.

I consider it as a very important goal to make more people engaged. Since you are making a ton of changes yourself, it's time to think about more general problems, like avoiding obvious single points of failure.

kocio-pl commented 4 years ago

There is also new kind of tool on GitHub, called actions, and one of them is greeting to the new contributors:

https://help.github.com/en/actions/automating-your-workflow-with-github-actions/about-github-actions https://github.com/gravitystorm/openstreetmap-carto/actions/new

However our biggest problem is that we don't attract new contributors in the first place, especially the ones that contribute more than once.

Adamant36 commented 4 years ago

it's time to think about more general problems, like avoiding obvious single points of failure.

A few things I've thought of while considering why I don't really contribute anymore that might be helpful (plus based on takeaways from being a collaborator over at the name-suggestion-index and dealing with new users).

  1. The lack of good code documentation. The combination of SQL with CSS is extremely complex at first look by people passing by. That could be greatly reduced by having inline documentation, or hell, just a simple file in the main directory stating what basic bits of code do. Another idea related to this is copying code into good first issues from PR's that are extremely similar. So people know almost exactly what to do ahead of time (I've it done in a couple of other repositories that have stronger communities to good effect).

  2. Some things should be taken out of the amenity file and put in separate files (like I had planned to do). It's better IMO to have more with less code then the reverse (way less new people contributed the name-suggestion-index when it was mostly one large file that was slow to load and took forever to search through then they do now with it split up into a bunch of different .json files that are much easier to manage individually).

  3. In the project file there should be spaces between the end of one id - and the start of another one. That way's it's not one long file of code. Which is kind of intimating and makes it harder to find things or tell where your at if you look away for a minute.

  4. More people will probably come along when the switch to vector tiles and z20 happens (if it does). A lot the PR's and discussion right now is for extremely technical stuff that doesn't really effect map rendering that much if at all, like changing things to ST_PointOnSurface. Which might give people the impression more low hanging fruit just isn't a priority and will be a distraction from the bigger picture issues currently being dealt with. There's a few minor PR's that @jeisenbe asked for feedback on a few months ago that never got a response, but then other more technical (or controversial PR's get immediate feedback and have a bunch of messages).

  5. Turn around on PR's is often way to long and there's to much discussion over minor (probably not ultimately important) things. Know one is going to do a PR for something if the impression that it will take 10 months for the PR to be merged while all the collaborators try to figure out if a slightly lighter shade of green or a .0001 inch width is better.

  6. It also helps if there isn't a big backlog of open PR's in my opinion. As it currently is someone opens a PR, you guys get half-way through debating it, then 3 more PR's are already open. @jeisenbe has 10 PR's open in the last month. The lower down ones are mostly being ignored at this point and have to re-based. Most of the PR's are not related to each other and there isn't really any underlining theme to it. Which just makes it look like you guys are all over the place. How is someone looking over the repository to decide if they want to contribute going to know what the current priority with the project is or what should be worked on? You guys should have a theme for each point release (bugs, cartography, whatever), pick some issues out of it, and make those the focus of what to work on. Right now the projects tab isn't useful at all and there isn't really any consorted effort to reach the milestones in any logical way. If you don't have a vision for what your moving toward as a project know one is going to be compelled to be a part of it or motivated to stick around IMO. Sometimes I get semi motivated to work on something and then I quickly lose my motivation because I don't see how it will improve the project besides just "fixing" something, and I don't want to fix something just for the sake of it. I'm sure other people feel the same way.

    1. More could be done to foster new contributors coming in and being retained. Good first issues is one aspect of that, but also asking people who open issues if they want to work on it themselves and helping them through it (something @kocio-pl does well to a degree, but could be done more consistently). You guys should look at Plots2's repository and how they foster new contributors and a community. Especially their three pinned issues. I think they have a good way of doing it. Although they are a different type of project, I think there is some good things that could be taken away from it. Like having mentors for new contributors and a basic issues just for how to setup their website by "working together." If you had a such an issue I'd totally help people go through the Docker setup and deal with other problems related to setting up the environment. I think that is something everyone should have down at least on a basic level before even working on an issue (maybe there should even be a simple "beginner" change to the rendering that they can be lead through before taking on something big in a real issue).

  7. Feature branches per https://github.com/gravitystorm/openstreetmap-carto/pull/3901#issuecomment-554720825 (I'd probably take over some stagnant PR's and work on them, but as it is it's kind of a hassle).

  8. Make the heading on the code tab more inviting and "active" looking.

  9. Add some Github topics (categories, labels, whatever they are called) to the code tab so people will be funneled in when looking through Github for things to work on. For instance "CSS", "CartoCSS", "Cartography", or whatever.

Anyway, those are my thoughts about it. Hopefully at least something in there is useful.

jeisenbe commented 4 years ago

Thank you for the detailed suggestions, @Adamant36

  1. good code documentation.

This is a good suggestion. Perhaps we could have a new document for new contributors called "DOCUMENTATION.md" or "DEVELOPMENT" or similar, which would explain how each file and piece of the rendering stack works together, and how changes can be made. It could be useful for people who want to adapt this style for their own purposes, as well as for contributors.

Adding more in-line comments about unusually pieces of code is also a good idea.

  1. ...take out of the amenity-points.mss file and put in separate files

Would you be interested in submitting a PR for this? An easy first step might be moving out #amenity-line and perhaps moving #trees to landcover? Another easy first step could be moving #text-poly-low-zoom and #text-point to a separate file (text.mss?), since many of these text labels are from features in landcover.

There are also chances to shorten the code of amenity-points by using cascading more efficiently.

  1. In the project file there should be spaces between the end of one id - and the start of another one

+1 good idea.

  1. Too much discussion / argument

This is difficult to fix. I don't think all of the maintainers have agreed on a particular vision for how this style should change, or even on what criteria we should use to approve or reject changes. See #3951 which unfortunately has only gotten comments from 2 other contributors so far, though there was a long history of discussion in issue #2436 before.

  1. It also helps if there isn't a big backlog of open PR's in my opinion. ...

Do you feel that if you submit a PR it might not be reviewed promptly? Or is it more of a concern that the maintainers might not come to consensus about accepting it?

Many active github projects have old PRs open for months and years. iD has 14 open PRs and 11 were opened more than 12 months ago. The openstreetmap.org website github has 46 open PRs: https://github.com/openstreetmap/openstreetmap-website/pulls

The lower down ones are mostly being ignored at this point and have to re-based

Sometimes the person who submitted the PR gets busy and does not have time to make improvements for a while. They have not been entirely ignored by the maintainers. If we look at the least recently updated PRs - 3 were last commented in September (between 30 and 60 days ago), 6 more were last commented on in 3 to 4 weeks ago, and the rest (27) have all be updated in the past 2 weeks, mostly in the past week. Only a

  1. feature branches as per https://github.com/gravitystorm/openstreetmap-carto/pull/3901#issuecomment-554720825

I think our current development model is basically the same as that link, except that we merge to "master" instead of to a separate "develop" branch and we tag releases differently?

How to fetch and check out another contributor's git branch is described here: https://github.com/gravitystorm/openstreetmap-carto/pull/3901#issuecomment-554737522

7 - 9 and 10 All good suggestions. Would you like to try setting these up?

jeisenbe commented 4 years ago

Do you find any of the current "good first issue" and "help wanted" labels interesting?

Would it help if we opened a new "2020 Road Map" issue or something similar to try to get some consensus on what to work on?

Adamant36 commented 4 years ago

his is a good suggestion. Perhaps we could have a new document for new contributors called "DOCUMENTATION.md" or "DEVELOPMENT" or similar

I think that would be helpful. A lot of this is pretty obscure.

Would you be interested in submitting a PR for this?

Unfortunately I was never able to get Kosmtik to run probably by removing things from the amenity file. It would always error out for some reason. I might give it another try though. If not, I could probably move the things you mentioned without much problems. I think they are good suggestions. I can also open new issues for the other stuff so they will be actionable to anyone that comes along.

There are also chances to shorten the code of amenity-points by using cascading more efficiently.

Do you have an example of that somewhere or can you do a PR for a small part of it and I will do the rest after that?

+1 good idea.

When I have some time I'll probably do a PR for the spaces in project file.

This is difficult to fix. I don't think all of the maintainers have agreed on a particular vision for how this style should change, or even on what criteria we should use to approve or reject changes.

I agree there. I think the first is to at least figure out the consensus based decision process. There are some things I think everyone agrees on though that I feel could be dealt with in the meantime without a grander vision being involved. For instance migrating to vector tiles and showing surfaces on roads. Getting some bigger picture things done for a while like those instead of spinning your tires on things you don't agree on will probably help in the long run. Even if this project completely sinks into the ground vector tiles and displaying road surfaces should still happen IMO. There will always be things you disagree about.

Many active github projects have old PRs open for months and years. iD has 14 open PRs and 11 were opened more than 12 months ago.

True. iD Editor only really gets PR's from two people. Who are both paid and have a shared vision. They don't have as much to balance as this project IMO either. I think it's different here because

  1. Everything is effected by everything else. So, if someone opens a PR for something and it waits to long, it risks never getting merged because the "rules of engagement" have changed. Like with the whole "to many icons" issue which reached a completely arbitrary point where the collaborators aren't adding new icons for things. Our whole "scrub, heath, whatever it was" issue a while back is a good example of that also. Various issues and PR's are very dependent on each other to a degree.

  2. It's really easy for there to be merge conflicts and the longer you wait to merge something the more likely it is that the conflicts won't or can't be worked out and that the person will have to ditch the PR and open a new one or not. I've had to do that several times. Even in cases where they don't it just creates extra hassle. I think it's respectful to contributors to have a semi-soft deadline on things sometimes and not spread your attention to thin or neglect certain things to the benefit of others. Also, it would help with the consensus process because it could someone from being able to filibuster indefinitely. There should be consequences for not taking action, like closing issues or doing something another way. That's just my opinion though.

Sometimes the person who submitted the PR gets busy and does not have time to make improvements for a while.

That's true. I think people just forget about things because there's a lot of other things taking up mind share. It helps to follow up with people to see where they are at after a PR has been sitting for a while. I think your fairly good at doing that. Although, other people don't seem to be good at responding. Which I understand. Just make following up a practice I guess.

I think our current development model is basically the same as that link, except that we merge to "master" instead of to a separate "develop" branch and we tag releases differently?

I haven't really looked into it that much, but I think there are some subtle but still important differences. I'll have to read up on it more though.

All good suggestions. Would you like to try setting these up?

I think with 7-10 those are things you have to do on your end.

Do you find any of the current "good first issue" and "help wanted" labels interesting?

Since you mentioned labels, something id suggest is adding descriptions for more of them. Currently only 2 out of 40 of them describe what their purpose is. It would help when people are browsing through them looking for new issues to work on.

Would it help if we opened a new "2020 Road Map" issue or something similar to try to get some consensus on what to work on?

I think that would be a good idea. It's hard to have a vision for the project when one hasn't been created. I know there's the guidelines or whatever, but I feel like that's different. Having a road map would allow you to focus on certain goals and not get bogged down in minor details that could probably be put off. The 3 main priorities for 2020 and where your energy should go IMO is vector tiles, road surfaces, dealing with the reload and the issues related to it. The style would be in a much better place after those things get done. There should really be clear next steps for all of them and a soft-timeline for when they can be accomplished. Even if it's something like "Switch everything to ST_PointOnSurface that needs to be by the middle of 2020" or whatever. Forgoing the timeline and just the steps clearly down for these things would even be a massive improvement. But last time I checked the reload at least was suppose to be this year. So it should be moving forward in some way at this point.

Sorry if that was long winded.

jeisenbe commented 4 years ago

There are also chances to shorten the code of amenity-points by using cascading more efficiently. Do you have an example of that somewhere or can you do a PR for a small part of it?

See: https://github.com/gravitystorm/openstreetmap-carto/blob/132f0d0938adb44fb9e2a5adee4ec313bcea3a51/amenity-points.mss#L1483-L1531

This could also be done for features that use the same color icons: eg all the @accomodation-icon lines (hotel, motel etc), or all of the @health-color features.

jeisenbe commented 4 years ago

labels, something id suggest is adding descriptions for more of them

@matkoniecz - I believe you might have started using the labels originally? Would you be interested in adding descriptions about the labels in github or describing how we should use them?

jeisenbe commented 3 years ago

Here is a tutorial about making your first PR, for future reference, designed for a Mac (or PC) user who has never used github or the command line before.

See the tag “good first issue” for some options of issues which are feasible for a first-time contributor to start with: https://github.com/gravitystorm/openstreetmap-carto/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22

If you have a Linux machine you can follow the step-by-step instructions in INSTALL.MD or see this tutorial by @Ircama for more details: https://ircama.github.io/osm-carto-tutorials/

If you have a Mac or PC you can use docker to set up a test rendering server on your own computer. This uses a program called Kosmtik and a copy of current Openstreetmap data to make test renderings with your proposed changes. Then you can make a Pull Request here in this Github repository to actually change the code. If the PR is accepted then your changes will eventually show up on places where this style us used, which currently includes the “standard" map layer on www.openstreetmap.org

For Mac and PC there is a Docker container option which keeps the map rendering software in a separate system, and several steps are automated for you. This is what I use, previously on my 2012 Macbook (now upgraded to 2015).

For this option, I'm pretending that you know nothing about git or the command line, because this was the state of my knowledge a few months ago.

1) download Docker and install it.

2) Next, create a "fork" of this repository on github, and set up git on your computer\

3) Clone the fork to your system: Open the Terminal application (Mac) or Command Prompt (PC). Change the directory to where you want the Openstreetmap files to go. (For example, create a folder called "osm" under your home directory, then type cd ~/osm/). Then enter git clone https://github.com/Your_User_Name/openstreetmap-carto.git - in my case this would be https://github.com/jeisenbe/openstreetmap-carto.git but you need to enter the actual name for your fork. You may be prompted to download command line tools or similar to use this command. This will make a new directory like ~/osm/openstreetmap-carto/ with all the information in this repository (about 30 or 40 megabytes?)

4) Download a file in the osm.pbf format for your area. Check https://download.geofabrik.de/ and https://download.openstreetmap.fr/extracts/ for options. You might want to pick a smaller province or small country if you are using an old computer with limited RAM and processing power. Alternatively you can download an osm.xml file of a small area directly from openstreetmap.org using the "export" function, or you can download data in JOSM and same as an .osm.pbf file. The name of the file should be changed to data.osm.pbf and it needs to be in the same directory ("folder"), eg '~/osm/openstreetmap-carto'

4) Now you are ready to set up everything on Docker. These next 2 commands will take a long time the first time you use them, and will download up to a Gigabyte of data, but the next time will be much faster.

Enter docker-compose up import in the command line (Terminal or Command Prompt)

This creates the whole Docker container and imports the openstreetmap data from data.osm.pbf into a special rendering database.

Once this is done, make a new tab or window and enter docker-compose up kosmtik - this starts up Kosmtik, the test rendering application. During this process nearly a gigabyte of data will need to be downloaded, but this only happens the first time.

5) View test renderings by opening an internet browser to http://localhost:6789 - or if you know exactly where you want to see, you can enter the zoom level, latitude and longitude like this: http://localhost:6789/openstreetmap-carto/#14/51.5000/-1.0000 - go find a place in your downloaded data that has a railway station.

This will show you how things look currently. But you want to change the icon for railway stations, so let's make a new branch:

6) In a new Command Prompt or Terminal window or tab, enter git checkout -b station-symbols - this creates a new "branch" called station-symbols where you can test different icons, and checks out the new branch.

7) Add your new SVG icons to your new openstreetmap-carto/symbols folder, for example railway_station1.svg and tram_stop1.svg. If you have a few different options, you can number them like railway_station1.svg and railway_station2.svg so you don't lose track.

7) Change the code for your PR. For this example, we are putting in a new railway station icon. Where is it? Try searching for "station" in the new /openstreetmap-carto folder - you'll find the code you want to change in station.mss:

  [railway = 'station'][zoom >= 12] {
    marker-file: url('symbols/square.svg');

Change marker-file: url('symbols/square.svg'); to marker-file: url('symbols/railway_station.svg');

(We should also change the railway halts and tram stops, but that can wait till later, once you decide if halts should use the same symbol.)

Once you've changed the code in a text editor, save the changes. Then go back to the Terminal (or Command Prompt) and enter: git add symbols/railway_station1.svg Then git commit -a -m 'Change icon for railway stations to railway_station1.svg'

This saves your changes to git, which will make it easy to fix things later and upload the changes to this github repository when you are ready.

8) Check out the test rendering by opening up the browser window to http://localhost:6789/openstreetmap-carto/ again and finding the railway station. Now if you are at the right zoom level, it will show your new icon.

If you don't see the changes, use the reload option, or go back to the command prompt / terminal window where you opened kosmtik (remember docker-compose up kosmtik) and type Ctrl-C to stop it, then use docker-compose up kosmtik to restart the test rendering - sometimes this is needed if you change an icon but don't change the name.

There's an option to "export" a png image from the Kosmtik test rendering, but it's best to take a partial screen shot, in most cases. Remember to name each test rendering with the details, like the zoom leve, the place tested and the name of the svg file and the date, e.g.: z15-kings-cross-railway_station1-2019-5-26

Now you can make more changes, for example, change the icon for [railway = 'tram_stop'] a few lines down, and also consider [railway = 'halt']. You can also do some more editing to the code so that the current square icon is used at lower zoom levels (z12 to z14) but the new icon is used at high zoom levels (>z15), eg:

[railway = 'station'][zoom >= 12] { marker-file: url('symbols/square.svg'); ... [zoom >= 15] { marker-file: url('symbols/railway_station1.svg'); This code will use the current symbol to render z12 to z14, but the new symbol for zoom level 15 and higher.

Remember to use git add symbol/name_of_your_icon.svg when you add a new icon in the code, and then use git commit -a -m '<description of your changes>' to record your work.

If you want compare the test rendering with the current rendering, you can switch branches: git checkout master to go back to the master (original) branch, and then git checkout station-symbols to switch to your railway station symbols testing branch.

It might also help to read: Docker.MD and CONTRIBUTING.md https://github.com/gravitystorm/openstreetmap-carto/issues/3782#issuecomment-495926741 - and also see this follow-up comment: https://github.com/gravitystorm/openstreetmap-carto/issues/3782#issuecomment-495957343

jeisenbe commented 3 years ago

Herea are a couple of things I wish I had known before making test renderings, although you may know this already, if you've used git and github before:

1) If you want to test several areas from different countries, the easiest option is to download the data in JOSM, and then save the file like stations.osm.xml in your openstreetmap-carto directory (folder). Then to import this for rendering, enter this command into the command line: docker-compose run -e OSM2PGSQL_DATAFILE=stations.osm.xml import (change stations.osm.xml to the actual name of the file. You can use .osm.xml or .osm.pbf files)

2) Sometimes it is much easier to test changes first with fake test data. After you get something that works and looks right, it can be tested on real data. @matthijsmelissen made this file, which I've updated: test.osm.xml.zip - But eventually the changes should be tested on real data before submitting the PR.

3) Before submitting a PR (pull request), it's helpful to rebase your branch against the upstream master branch:

1st time only: git remote add upstream git://github.com/gravitystorm/openstreetmap-carto.git

To rebase, run: git fetch upstream Then git rebase upstream/master

If you've made lots of commits, you might want to clean up the history with an interactive rebase by "squashing" the commits that you didn't end up using (eg testing other icons or rendering styles that you later rejected):

git rebase -i upstream/master

Then you can push your branch, and you will be given an URL to open which will start a PR:

git push --set-upstream origin name-of-your-branch

But, don't use git rebase after you have already opened a pull request from the branch.

4) When opening a Pull Request, you will usually need to show a couple of test images as examples, and it's helpful to have a link to the location in the current openstreetmap.org rendering. Because of this, it's important to label your test rendering files carefully, if you make more than a couple.

5) Many of the maintainers (the users with the "collaborator" label next to their user name) here have a rather direct communication style. You may find that the criticism and comments are too direct or impolite, but remember that many of the maintainers are not writing in their first language and are used to a different way of communicating.