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

Changeset comment list on the profile #842

Open
HolgerJeromin opened this issue Nov 20, 2014 · 41 comments

Comments

Projects
None yet
@HolgerJeromin
Copy link
Contributor

commented Nov 20, 2014

If there is a changeset comment i get an email. The link in this email is the only place to get to it.
Right now i have no list of changesets with comments from my subscriptions. This could be in the top row of my profile.
With this the link "Meine Kommentare" (german for "my comments") should be more precice, that this are diary comments and not changeset comments.

@ukasiu

This comment has been minimized.

Copy link
Contributor

commented Jan 2, 2015

I've prepared something addressing this issue: you can test it at http://comments.apis.dev.openstreetmap.org/changeset_comments (more comments lists available after registration). Suggestions are welcome (e.g. where to put links to the new views, how to style the table), I'll submit a pull request in few days.
image

@pnorman

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2015

where to put links to the new views

In keeping with others, we'd link them on the user's page, like we do for "Edits", "Map Notes", etc.

This would probably give a URL for changeset discussions like http://www.openstreetmap.org/user/TomH/discussions

We're also calling them changeset discussions, not comments, because the word comments is already used to describe the value of the comment tag on the changeset.

@polarbearing

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2015

Yes that is urgently needed now. I'd like to follow up on discussions I started,
in particular if there is no response, but there is no way to trace them.

@ukasiu - is that accessing real discussions or just a mockup list?
Is the login on the dev server with my real OSM account or a test registration?
I'm reluctant to use my OSM password as there is no SSL nor OAuth.

@tomhughes

This comment has been minimized.

Copy link
Member

commented Jan 12, 2015

All dev server sites are completely separate, with their own database.

@ukasiu

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2015

@polarbearing I'll try to finish it this week, but I can't promise anything.

@polarbearing

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2015

@ukasiu - any news :-) ?

@ukasiu

This comment has been minimized.

Copy link
Contributor

commented Mar 10, 2015

@polarbearing, sorry, I forgot about the whole thing and didn't notice the email. I'm gonna work on it today/this week.

@ukasiu

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2015

I made links at discussions list page more consistent (http://comments.apis.dev.openstreetmap.org/user/TomH/changeset_comments/received) and moved link to discussions from main nav to user profile (http://comments.apis.dev.openstreetmap.org/user/ukasiu).

@ukasiu

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2015

Changing url part from changeset_comments to discussions would cause an inconsistence because of e.g. post 'api/0.6/changeset/comment/:id/unhide'. @tomhughes what do you think?

@tomhughes

This comment has been minimized.

Copy link
Member

commented Mar 11, 2015

I think the link on the user profile should probably be "Changeset Discussions" as "Discussions" is very generic.

As for the URLs well we could always change the hide/unhide URLs as I doubt anybody is using them other than the web site itself - we can always redirect the current ones if we really want.

@ukasiu

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2015

Ok, so I changed the labels and urls, ukasiu@a279753 . Any suggestions on how to display discussions (now it's ugly styled table)?

@HolgerJeromin

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2015

the old diary "comment" could be renamed, too. To be more different to the new feature.

@ukasiu

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2015

Hmm, I think it's a bit beyond the scope of this feature.

@tilmanb

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2015

I would prefer this was added now to the website, and questions of beautifying the discussion display addressed later. The feature is really missing!

@tomhughes

This comment has been minimized.

Copy link
Member

commented Jun 25, 2015

Add what? No code has been offered yet so there is (as yet) nothing to add!

@simon04

This comment has been minimized.

Copy link
Member

commented Sep 1, 2015

Here are the code changes: master...ukasiu:comments_list

@ukasiu What has to be done (besides visual table improvements)?

@stephan75

This comment has been minimized.

Copy link

commented Oct 31, 2015

@ukasiu ... is there anything that we can do as normal OSM users to get some progress on this really fine draft of feature enhancement? more testing needed in the demo instances mentioned above? What is needed for a pull request to the main repository?

@polarbearing

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2016

If @ukasiu does not have the time to complete a PR here, would another developer be able to finish this valuable feature?

@gileri

This comment has been minimized.

Copy link

commented Jan 28, 2016

With @tuxayo we worked to make @ukasiu's code merge with current codebase and pass the tests. We are currently stuck due to a lack of time on verifying whether the ukasiu's original tests are throrough enough to allow a PR now.

master...gileri:comments_list

EDIT : Change link as per @simon04 comment

@simon04

This comment has been minimized.

Copy link
Member

commented Mar 18, 2016

@gileri: Any update on this? Is any help needed?

Btw: this shows the right comparison of the changes: master...gileri:comments_list

@tuxayo

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2016

Help to review the state of the branch would be needed.
I don't know enough the context to have an idea of the completeness of the work.

@stephankn

This comment has been minimized.

Copy link

commented Feb 28, 2017

Is it possible to merge this before the code base diverges too much? finding your own changeset comments is quite useful to follow up on those not getting any response.

And would be quite sad to see the effort to create the patch being wasted.

@tomhughes

This comment has been minimized.

Copy link
Member

commented Feb 28, 2017

There's still nothing to merge is there?

@stephankn

This comment has been minimized.

Copy link

commented Feb 28, 2017

The code change is there, right? Even not being fluent in ruby, this looks like it implements a display of a users comments: master...gileri:comments_list

@tomhughes

This comment has been minimized.

Copy link
Member

commented Feb 28, 2017

A random set of commits in a random persons repository is not a merge proposal though.

A merge proposal is a PR of those commits opened by somebody who is ready to resolve any review comments.

@stephankn

This comment has been minimized.

Copy link

commented Feb 28, 2017

No arguing about the PR process on Github. But back to my original question: Is it possible to get these changes into a condition allowing them to be merged? Before it was stated that help is needed reviewing the branch. Who can do this and possibly provide feedback to the original authors on what to change to get it ready? @tomhughes What is your expectation to the source before accepting a PR? What sort of tests are required?

@tuxayo

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2017

@stephankn I think someone needs to resync (rebase or merge) with master and make a new PR before getting feedback.

And also doing a basic manual check if it's still working.

@maracuja-juice

This comment has been minimized.

Copy link

commented Sep 18, 2017

Any news on this? This feature would be very valuable.

@paulgillard

This comment has been minimized.

Copy link

commented Sep 22, 2017

I'm going to try and work this up to a pull request. Reading the work that's been done so far my first query is whether we need pagination for changeset comments. Happy to keep it in if needed but no point in over engineering if the data doesn't call for it. Currently pagination will kick in at 20 comments.

@tomhughes, with my being new here, your the only one I can surmise has production db access. Can the db be queried to see how many changesets have more than 20 associated comments?

@woodpeck

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2017

@paulgillard great you're looking into this. There might be a small misunderstanding - this is about displaying all changeset comments that (a) a user has made to other changesets, (b) a user has received on their changesets, or (c) all comments on changesets the user has subscribed to. None of this is limited by how many changeset comments there are for one single changeset. There are certainly users who have sent 100s of changeset comments, and hopefully less who have received 100s of them ;)

@tomhughes

This comment has been minimized.

Copy link
Member

commented Sep 22, 2017

There are currently 41 changesets with more than 20 comments and the all time winner is https://www.openstreetmap.org/changeset/51820940 with 107 comments.

As @woodpeck says that isn't really the question here. The answer to the real question is that 859 users have more made more than 20 comments with the winner having 6998 in total!

@paulgillard

This comment has been minimized.

Copy link

commented Sep 29, 2017

I've just created a pull request (#1642) for this issue. I'm happy to discuss and act upon feedback to get this issue completed.

In reference to the conversation further up in this issue, I've changed the naming back to Changeset Comments from Changeset Discussions. In the new views what you see is a selection of comments from different discussions and so Changeset Comments feels more appropriate IMO. A discussion is all the comments on a single changeset in chronological order. Such as at https://www.openstreetmap.org/changeset/51820940. @tomhughes and @woodpeck, the term discussions is what caused my confusion above.

I'm not entirely satisfied with the secondary links under the main heading on the new pages. They aren't exactly concise. Suggestions welcome.

@pnorman

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2017

The reason they're discussions is changeset comments are what the author of the changeset adds with the comment tag.

@paulgillard

This comment has been minimized.

Copy link

commented Sep 30, 2017

I understand where you're coming from with naming this feature discussions as creating some difference between the two would be ideal. But we already:

  • have a ChangesetComment model
  • have comment, hide_comment, unhide_comment and comments_feed actions on the changeset controller
  • have the term 'comment' in appropriate places on the view for the changeset action on the browse controller. The term 'discussion' is only used on this view as a title for the set of comments. See https://www.openstreetmap.org/changeset/52370342
  • have diary comments and note comments and keeping this as changeset comments retains some consistency among these parts of the site.

To my mind the ideal solution would be to rename the comment written by the author when the changeset is created to something like remark instead. But I quite understand that isn't happening in a hurry. My point being though that if it were remark I don't think anyone could argue that naming this feature changeset comments was not the best approach.

If the majority prefer changeset discussions then so be it. But I feel it's better to keep to the naming the site has already established in place despite the overlap. If it becomes too confusing for users once they actually start using it then that problem can be solved on another ticket. But it might be that it won't confuse.

@woodpeck

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2017

I agree that the naming is a bit unfortunate. We stumbled into this because we were all used to having "commit comments" in software revision control systems, so we thought it would be appropriate to call what you specify when you upload data a "changeset comment". Colloquially we say "I commented (on) his changeset" and then what we really did was not place a changeset comment but a discussion entry.

(I can't help but notice that what I am writing here, in GitHub parlance, is also a "comment".)

I would welcome it if we could improve this, although "remark" to my mind would not be an improvement - it sounds too casual. Something like "statement" would capture the importance better but still I don't like it. Also, I fear that the term "changeset comment" is meanwhile so widely used (in lots of introductory material, editors, wiki etc., and also as @pnorman pointed out, the tag is called "comment") that the ship might have sailed.

Perhaps we could weasel around the issue by calling things "the initial changeset comment" (or "the uploader's changeset comment") and "other changeset comments" ;)

@Zverik

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2017

For that reason I suggested renaming them to changeset discussions when they were implemented.

@pnorman

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2017

For that reason I suggested renaming them to changeset discussions when they were implemented.

I thought we were reasonably consistent in calling them that. The names of models and variables in the code isn't user-facing.

@Codain

This comment has been minimized.

Copy link

commented Nov 29, 2018

Any news on this one? I commented some changesets and am unable find them since the author did not answer. I'm also looking for this feature in order to track the comments I made.

@tomhughes

This comment has been minimized.

Copy link
Member

commented Nov 29, 2018

If there was news then this would be closed.

@Piskvor

This comment has been minimized.

Copy link

commented Nov 29, 2018

@Codain: There's a tool to list your changeset discussions, but I don't think it's accessible as a direct link:

  • log in using OSM at https://hdyc.neis-one.org/
  • search your username
  • there's a link "created N comments" approximately halfway down the page
  • (not sure how far into user history this goes)
@Codain

This comment has been minimized.

Copy link

commented Nov 29, 2018

@Piskvor Thank you! I was on my way to put this link on the issue but you did it faster. This website is not that ergonomic for newbies but it gives lot of services and this one on comments is very valuable. At least it can help while a handy contributor (@paulgillard ?) works on the "news".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.