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

Include hashtags in notes #3938

Closed
wants to merge 2 commits into from
Closed

Include hashtags in notes #3938

wants to merge 2 commits into from

Conversation

angoca
Copy link

@angoca angoca commented Feb 22, 2023

This allows to identify notes created from website, from other notes, which is useful when filtering and solving notes.

@tomhughes
Copy link
Member

If we're going to do this then we should do it properly as meta data on the model and not by expecting people to guess the meaning of bits of the note body.

@gravitystorm
Copy link
Collaborator

@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. 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 - 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.

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.

3 participants