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: install @nextcloud/prettier-config, eslint-config-prettier #6510

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

luka-nextcloud
Copy link
Contributor

📝 Summary

  • Install @nextcloud/prettier-config
  • Install eslint-config-prettier
  • Format code with prettier

Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this @luka-nextcloud

I'm all for standardizing things and linting them. I'll point out a few things that i am not so sure about though.

Comment on lines +240 to +241
(t) =>
t.status === STATUS_SCHEDULED || t.status === STATUS_RUNNING,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like splitting the arrow function like this. Maybe just personal taste.

Comment on lines -11 to +22
return s.toString()
.split('&').join('&')
.split('<').join('&lt;')
.split('>').join('&gt;')
.split('"').join('&quot;')
.split('\'').join('&#039;')
return s
.toString()
.split('&')
.join('&amp;')
.split('<')
.join('&lt;')
.split('>')
.join('&gt;')
.split('"')
.join('&quot;')
.split("'")
.join('&#039;')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer the previous version as these calls come im pairs of split(..).join(...) and that's easier to see in the original.

Normally I would agree that it's a good idea to put every function call on a new line... But here I don't. So I wonder if the rule that enforces this is really a good idea.

Comment on lines -17 to +19
<NcActionButton v-for="type in taskTypes"
<NcActionButton
v-for="type in taskTypes"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think pretty much everywhere else in nextcloud we have the first attribute on the line with the tag name just like this was before.

I actually like the separation - but I care about consistency more. So not sure what best to do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🧭 Planning evaluation (don't pick)
Development

Successfully merging this pull request may close these issues.

stylelint broken after migrating from webpack to vite
2 participants