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

checkpatch.pl warning about non-permalinks to Zulip #1104

Open
ojeda opened this issue Aug 21, 2024 · 13 comments
Open

checkpatch.pl warning about non-permalinks to Zulip #1104

ojeda opened this issue Aug 21, 2024 · 13 comments
Labels
easy Expected to be an easy issue to resolve. good first issue Good for newcomers medium Expected to be an issue of medium difficulty to resolve. • misc Related to other topics (e.g. CI).

Comments

@ojeda
Copy link
Member

ojeda commented Aug 21, 2024

Please see https://lore.kernel.org/rust-for-linux/CANiq72mrwBs_YTcBvge4ME5bwSLKbNaoFU+KZw3EfCTyGjiJ9w@mail.gmail.com/.

Please note that the checkpatch.pl maintainers will need to agree to the change.

Zulip apparently has support for permalinks incoming (or already present but not exposed through the UI). It would be ideal if the warning shows the correct link to use (if that is possible to infer from the other link) or at least point to the documentation (possibly https://chat.zulip.org/api/construct-narrow#narrows-that-use-ids) or how to do it in the UI (assuming it is ready in Zulip).


This requires submitting a proper patch to the LKML and the Rust for Linux mailing list. Please recall to test your changes (including generating the documentation if changed, running the Rust doctests if changed, etc.), to use a proper title for the commit, to sign your commit under the Developer's Certificate of Origin and to add a Suggested-by: tag and a Link: tag to this issue.

@ojeda ojeda added • misc Related to other topics (e.g. CI). good first issue Good for newcomers labels Aug 21, 2024
@BiscuitBobby
Copy link

Hello, I would like to give it a shot.

@ojeda
Copy link
Member Author

ojeda commented Aug 22, 2024

Sure, please go ahead!

@BiscuitBobby
Copy link

BiscuitBobby commented Aug 23, 2024

Hey, wanted to clarify a few things:

  • This includes all the subdomains of zulip.org and zulipchat.com and not just chat.zulip.org
  • Stream links and message links are perma-links, ie., link should look like:
    https://.../#narrow/stream/<anything>/ 
    
    or
    https://.../#narrow/stream/<anything>/topic/<anything>/<with or near>/<number>
    

@ojeda
Copy link
Member Author

ojeda commented Aug 24, 2024

  • This includes all the subdomains of zulip.org and zulipchat.com and not just chat.zulip.org

Hmm... If there are actual links that use those domains, I guess yes. However, I have only seen links like *.zulipchat.com, so unless we know they are used/generated by Zulip, I think it would be best to avoid them.

  • Stream links and message links are perma-links, ie., link should look like:

I am not sure what the new perma-links they are introducing are, but the ones we used so far have been always of the form:

https://rust-for-linux.zulipchat.com/#narrow/stream/x/topic/x/near/455637364

In order to shorten them, I usually replace the stream and topic name and leave an x there as above, since what counts is the number at the end.

If there is no number, and only a stream/topic name, then I think they are not perma-links, because if those change, the link is broken.

I would suggest testing the links to see if they are actual permalinks or not -- if needed, please feel free to test in our Zulip (or perhaps in a test instance they may have) creating a topic and then renaming it or similar.

Thanks!

@bjorn3
Copy link
Member

bjorn3 commented Aug 24, 2024

chat.zulip.org is the zulip for development of zulip itself.

@BiscuitBobby
Copy link

Thanks for clearing it up for me, will test links from rust-for-linux.zulipchat.com

@BiscuitBobby
Copy link

I've sent in a patch to the mailing lists.

@ojeda
Copy link
Member Author

ojeda commented Aug 26, 2024

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Aug 27, 2024
Zulip links to https://rust-for-linux.zulipchat.com can break in
case of renaming the topic or channel if they are not message
links (which are permanent links).

If a non-permanent Zulip link is referenced then emit a warning
and direct the user to the Zulip documentation.

Permanent links are of the format:
https://.../#narrow/stream/x/topic/x/near/<numerical_id>

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1104
Signed-off-by: Siddharth Menon <[email protected]>
@metaspace
Copy link
Collaborator

I don't understand how to use this feature. Is there a button somewhere, or do I have to manually write out something like this https://rust-for-linux.zulipchat.com/#narrow/stream/x/topic/x/with/465040682 ?

For getting the message ID, am I supposed to cut it from the URL?

@bjorn3
Copy link
Member

bjorn3 commented Aug 28, 2024

If you hover over a message and click on the three dots below each other, one of the menu options is "Copy link to message".

@ojeda
Copy link
Member Author

ojeda commented Aug 28, 2024

That is what I do, yeah. I think Zulip is implementing something similar for topics too.

@metaspace
Copy link
Collaborator

If you hover over a message and click on the three dots below each other, one of the menu options is "Copy link to message".

That would give me a link to a specific message. I think that if I want a permalink to a topic, I have to first get the link to the message and then rewrite the URL, right?

@ojeda
Copy link
Member Author

ojeda commented Aug 28, 2024

No, it is a permalink; however, it would point to the first message, yes.

That is what I use -- I think it is good enough until they provide permalinks to topics.

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Aug 28, 2024
Zulip links to https://rust-for-linux.zulipchat.com can break in
case of renaming the topic or channel if they are not message
links (which are permanent links).

If a non-permanent Zulip link is referenced then emit a warning
and direct the user to the Zulip documentation.

Permanent links are of the format:
https://.../#narrow/stream/x/topic/x/near/<numerical_id>

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1104
Signed-off-by: Siddharth Menon <[email protected]>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Aug 28, 2024
Zulip links to https://rust-for-linux.zulipchat.com can break in
case of renaming the topic or channel if they are not message
links (which are permanent links).

If a non-permanent Zulip link is referenced then emit a warning
and direct the user to the Zulip documentation.

Permanent links are of the format:
https://.../#narrow/stream/x/topic/x/near/<numerical_id>

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1104
Signed-off-by: Siddharth Menon <[email protected]>
@ojeda ojeda added easy Expected to be an easy issue to resolve. medium Expected to be an issue of medium difficulty to resolve. labels Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy Expected to be an easy issue to resolve. good first issue Good for newcomers medium Expected to be an issue of medium difficulty to resolve. • misc Related to other topics (e.g. CI).
Development

No branches or pull requests

4 participants