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

Maintenance update #27

Merged
merged 20 commits into from
Nov 19, 2023
Merged

Maintenance update #27

merged 20 commits into from
Nov 19, 2023

Conversation

KristjanESPERANTO
Copy link
Contributor

The PR does not contain any functional updates. Roughly summarized, these are updates, typos and format harmonization.

See the commits for details.

Thanks for this nice module! 🙂

@nkl-kst
Copy link
Owner

nkl-kst commented Nov 12, 2023

Thanks @KristjanESPERANTO for this PR and your work! There are many and I don't think it's necessary to change all these formatting aspects. Could you try to find a Prettier configuration that doesn't result in so many changes?

E.g. trailing comma in objets/arrays should be mandatory. There should be no whitespace after the function-keyword. No blank lines after function opening and so on.

@KristjanESPERANTO
Copy link
Contributor Author

I know it's a bit difficult to keep track of the changes when there are so many in one PR. Sorry for that.

@nkl-kst
Copy link
Owner

nkl-kst commented Nov 14, 2023

I read about Prettier and found out that it is "opinionated" with very few options. I think that's reasonable as "the biggest reason for adopting Prettier is to stop all the on-going debates over styles". Sounds good.

I'd like to honor your work and therefore stop the discussion about whitespaces and blank lines to accept this PR, but there are two more things that are important for me:

  1. We should add Prettier to the GitHub workflow
  2. Is it possible to split arrays over multiple lines, even if they have just one element (eg. in getScripts and getStyles)? I really like small diffs and this would improve the readability for changes when adding/removing array elements.

Thanks again for your work! In the last years I was busy with other things, but maybe I should also spend some time to improve this module, as it seems to be useful for some people :)

@KristjanESPERANTO
Copy link
Contributor Author

  1. Yes, that makes sense, I have done a similar thing in other projects in combination with husky and eslint. To avoid overloading this PR even more, I would tackle this in another PR. Or what do you think?
  2. I added // prettier-ignore to preserve the array format. I found no better option.

@nkl-kst nkl-kst merged commit 2584206 into nkl-kst:master Nov 19, 2023
0 of 2 checks passed
@nkl-kst
Copy link
Owner

nkl-kst commented Nov 19, 2023

Tested on my mirror and works like a charm! Thank you very much @KristjanESPERANTO 👏

@KristjanESPERANTO
Copy link
Contributor Author

You're welcome!

BTW, MMM-OnThisDay is now displayed very well on the module page that I am building.

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.

2 participants