[openstreetmap/openstreetmap-website] Include hashtags in notes (PR #3938)
Andy Allan
notifications at github.com
Wed Feb 22 15:22:49 UTC 2023
@angoca thank you for your pull request.
I agree with @tomhughes on this, and I have several other objections to this approach, as [I've commented in the related issue](https://github.com/openstreetmap/openstreetmap-website/issues/3932#issuecomment-1440222022). So I'm going to decline this pull request.
I'm going to review this PR anyway, with the aim of helping you with making future PRs. So here are my notes:
* I'm not sure that `#website` is particularly descriptive, especially if other hashtags are in use for other reasons than just marking what creating the notes (e.g. if there are hashtags for topics or challenges). It doesn't clearly indicate which website created it, for example, and other websites might start using `#website` too. Instead I would consider using the "generator" from our configuration, or perhaps something from the translations (`layouts.project_name`?) that is a) more descriptive b) more precise and c) likely to be configured differently for other installations of the software.
* If we are formally adding a structure to what was previously free-form text, we would need to indicate some kind of parsing rules for applications that want to consume it. For example, what characters are valid in a hashtag, what characters delineate one, etc.
* From [CONTRIBUTING.md#pull-requests](https://github.com/openstreetmap/openstreetmap-website/blob/master/CONTRIBUTING.md#pull-requests) - please don't include fixup commits. One of the commits in this PR is entirely replacing the other, so they should be combined together (using e.g. `git rebase -i`) into a single clean commit.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/3938#issuecomment-1440247404
You are receiving this because you are subscribed to this thread.
Message ID: <openstreetmap/openstreetmap-website/pull/3938/c1440247404 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20230222/e31de753/attachment-0001.htm>
More information about the rails-dev
mailing list