Skip to content

Fix Content component name to be eslint compliant #3050

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.

Sign up for GitHub

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

Jump to bottom
Closed
nickvergessen wants to merge 1 commit into from
Closed

Fix Content component name to be eslint compliant #3050

nickvergessen wants to merge 1 commit into from

Conversation

@nickvergessen
Copy link
Contributor

@nickvergessen nickvergessen commented Aug 16, 2022

Signed-off-by: Joas Schilling coding@schilljs.com

@nickvergessen
Fix Content component name to be eslint compliant
db8a417 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen added 3. to review Waiting for reviews 💥 breaking PR that requires a new major version labels Aug 16, 2022
@nickvergessen nickvergessen added this to the 6.0.0 milestone Aug 16, 2022
@vinicius73
Copy link
Contributor

vinicius73 commented Aug 16, 2022

Why not NCContent? #2643

@nickvergessen
Copy link
Contributor Author

nickvergessen commented Aug 16, 2022 •
edited
Loading

Because the Button was renamed to ButtonVue, so I just followed the pattern

@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Aug 16, 2022

Why not NCContent? #2643

I would also find it sensible to fix it all together with the other component names. An NC prefix seems to make sense, but if we apply this to all components it creates a lot of friction when updating to v6 (and changing all component names here). So I am not all set with this solution. ContentVue would be in line with e.g. ButtonVue, but that anyway was a not really discussed change, so as long as it is consistent, I am good with whatever we chose.

@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Aug 16, 2022

Input and consensus from @nextcloud/vuejs is required here, I think.

@skjnldsv
Copy link
Contributor

skjnldsv commented Aug 16, 2022

If to choose, I would go for NcContent too :)
Note the lowercase c because it's Nextcloud and not NextCloud 🙈

@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Aug 16, 2022

Let's do a little poll 😆

  • ❤️ = NcContent
  • 🚀 = ContentVue

Like and edit my comment if necessary.

@raimund-schluessler raimund-schluessler mentioned this pull request Aug 17, 2022
Merged
@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Aug 18, 2022

Superseded by #3058.

@raimund-schluessler raimund-schluessler deleted the fix-eslint-component-names branch August 18, 2022 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raimund-schluessler raimund-schluessler Awaiting requested review from raimund-schluessler

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

Assignees

No one assigned

Labels

3. to review Waiting for reviews 💥 breaking PR that requires a new major version

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@nickvergessen @vinicius73 @raimund-schluessler @skjnldsv