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

chore(comments): Drop dead code from comments app #42174

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Dec 12, 2023

Summary

Drop code that is not used anymore from repository - also drop "comments" script / entry point which is unused.

Checklist

@susnux susnux added 3. to review Waiting for reviews technical debt labels Dec 12, 2023
@susnux susnux added this to the Nextcloud 29 milestone Dec 12, 2023
@susnux susnux requested review from skjnldsv, Pytal, a team, emoral435 and ChristophWurst and removed request for a team December 12, 2023 13:42
@susnux susnux requested a review from artonge December 12, 2023 16:50
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the use of this script?

apps/comments/.l10nignore Show resolved Hide resolved
@susnux
Copy link
Contributor Author

susnux commented Dec 13, 2023

Drone (samba) unrelated

@susnux susnux merged commit 11e27ac into master Dec 13, 2023
49 of 50 checks passed
@susnux susnux deleted the chore/comments-cleanup-dead-code branch December 13, 2023 10:33
@susnux
Copy link
Contributor Author

susnux commented Dec 13, 2023

What was the use of this script?

Old API that was dropped long time ago it seems. The only API used by other apps is the OCA.Comments.View used by e.g. announcement center

@artonge
Copy link
Contributor

artonge commented Dec 13, 2023

I remember @nickvergessen was also afraid to lose an API for Talk

@nickvergessen
Copy link
Member

nickvergessen commented Dec 13, 2023

Old API that was dropped long time ago it seems

It was not dropped. I prevent removal just last month....

This is public API:

/**
* Load the Comments app into the page
*
* @since 21.0.0
*/
public function load(): void {
$this->initialStateService->provideInitialState('comments', 'max-message-length', IComment::MAX_MESSAGE_LENGTH);
Util::addScript('comments', 'comments-app');
}

Can we just keep it for now?

We can deprecate and drop in 3 years if there are reasons apart from "just because"

@nickvergessen
Copy link
Member

See #41491 (review) and further

@nickvergessen
Copy link
Member

Reverting in #42207 to restore the nice functionality and public API

@susnux
Copy link
Contributor Author

susnux commented Dec 13, 2023

@nickvergessen that is a different script that is still provided. comments-app is still provided.

Meaning the public API is still there just some dead files are removed.

@blizzz blizzz mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants