Config Updates

[selfish]

Background and Notable Changes:

• Add .editorconfig
• Update eslint config and ignore
• Update license
• Update dev html to match config
• Move to yarn and upgrade dependencies
• Update ts and rollup configurations
• Update hacs.json
• Add .tool-verions for package manager support
• Add a proper PR template
• Update GH workflows to auto-release on push to main

[selfish]

Hey @avataar,

I noticed you might have seen the new PR with all the config updates and stuff. I'd love to get your thoughts on it! You can check it out and drop any comments or suggestions, pull the branch and tweak things yourself if you like, and of course let me know if you think something could be done differently or better.

I'm totally open to different ways of doing things, so don't hold back!

Just a heads-up, the CI is failing right now because we don't have any releases yet, and maybe due to the repo name issue. But it's not related to the content itself, and everything seems to work on this branch.

When we merge it, a release will be created automatically, and that should fix the CI stuff. But if you have other ideas, feel free to share.

Looking forward to hearing from you!

Cheers!

[ThomDietrich]

@avataar I would appreciate your feedback as I have almost no experience with js/ts toolchains.

@selfish your PR is aimed against the "main" branch. Shall we merge this one first, then merge the develop branch?

[selfish]

@avataar I would appreciate your feedback as I have almost no experience with js/ts toolchains.

@selfish your PR is aimed against the "main" branch. Shall we merge this one first, then merge the develop branch?

I think either order of merging is acceptable, but this PR is quite opinionated and requires a thorough review and input (probably from @avataar). Additionally, @avataar can suggest the best approach for the merge order, as I believe we should integrate his work and fixes from the develop branch.

I don't mind reworking this after merging @avataar's work if that's easier.

[avataar]

It doesn't build for me because of lint errors:

/Users/avataar/tmp/tmp/lovelace-sun-card/src/cardContent.ts
  1:1  error  Run autofix to sort these imports!  simple-import-sort/imports

/Users/avataar/tmp/tmp/lovelace-sun-card/src/index.ts
    1:1   error    Run autofix to sort these imports!  simple-import-sort/imports
  152:65  warning  Forbidden non-null assertion        @typescript-eslint/no-non-null-assertion
  175:22  warning  Forbidden non-null assertion        @typescript-eslint/no-non-null-assertion
  193:26  warning  Forbidden non-null assertion        @typescript-eslint/no-non-null-assertion
  197:9   error    Unexpected console statement        no-console

✖ 6 problems (3 errors, 3 warnings)
  2 errors and 0 warnings potentially fixable with the `--fix` option.

I was able to run eslint --fix and removed the offending console.error() call and then it worked (but still shows the warnings).

Regarding the merge order I'm not sure if you're aware that the develop branch (from the original project, not just my fork) has lots of changes that aren't present in the main branch (including to some of those build files that @selfish changes with this PR). Thus I believe we should ditch the existing main and rename develop to main, then rebase this onto the new main.

[selfish]

Makes sense. How about we do this:
You create dev here as you did in your repo.
I'll then rebase this on your develop, fix lint errors, etc. We'll merge this to your dev, see that everything is in order, then merge all to main?

[selfish]

Also, sorry for lraving the lint errors in there, I neglected to push fixes for these

[ThomDietrich]

Hey @selfish,
isn't that overly complicated? I am with @avataar on this: Let's just merge develop, then merge your PR. After that @avataar can create a PR for his changes.
Let's take one step at a time.

[avataar]

I opened a PR from my develop to this develop - no big changes there so order shouldn't matter.

After we end up with a single up-to-date main branch here I'll open another PR with some other improvements that affect the failing tests.

[selfish]

Sure, go ahead!

[avataar]

I've closed my original PR that combined my old 3 PRs and opened 3 new PRs instead so it's clearer what/why.

[ThomDietrich]

Hey @selfish,
I've merged the develop branch and @avataar 's smaller PRs. We are now ready for your toolchain changes here. Hope resolving conflicts isn't that difficult. Maybe it would be easier for you to start with a fresh branch? Cheers!

[edwardtfn]

It doesn't build for me because of lint errors:

It built for me without any changes (other than the ones related to the name that I just discovered you have done before me). I have the lovelace-sun-card.js in another PR. I think we should prioritise uploading that file as it is required to the hacs validation (which is required to add to HACS default repository list).

[selfish]

I'll take care of it soon. I'll make sure everything is resolved, we have a successful build, and we release with a changelog.

[selfish]

@ThomDietrich @avataar - ready for review.

[selfish]

Note that once this PR is merged, release v0.2.0 will automatically be created, and HACS action will no longer complain about structure.

[avataar]

It's starting to look really good. I resolved most of the suggestions and left some new comments. Here's a summary of what's left to resolve before we can merge this:

• A bunch of downgraded versions in package.json (see comment)
• Many (but not all) .ts files seem to have been relinted with semicolons/no spaces after function names -- it's certainly a matter of debate which style to use but this goes way beyond "config updates" and we lose history. I strongly suggest you revert those. I'm guessing that was unintentional.
• Unexpected include from node_modules in tsconfig.json
• .eslintrc.cjs (new, used now) vs .eslintrc (old, unused) - the old file should be removed and maybe you should review some key differences between the two files. Notably, the old one has rules that would've prevented the relinting of all those .ts files:

    "semi": ["error", "never"],
    "space-before-function-paren": ["error", "always"],

[avataar]

@selfish decided to work on his own fork instead, so I'll fix the remaining issues.

[ThomDietrich]

@avataar amazing, yes please do! Thanks! Users are getting nervous and it would be great if we could publish a version asap

[edwardtfn]

Anything I could help? I'm a it lost about which branch and what is still remaining to do.

[avataar]

Everything should be OK now. If anyone wants to have a final look at the changes I'd appreciate it.

[ThomDietrich]

All good from my side. @avataar imho you are imho good to merge

[edwardtfn]

Let's merge so we fix any remaining issues required for building (and test in HACS). Cosmetics can come later.

[edwardtfn]

file: "./dist/lovelace-sun-card.js",

Is this correct? On dev/index.html you removed the reference to folder /dist.

[avataar]

I think it's just a remnant from this file originally being a copy of the prod rollup config, while the relevant part is the serve section. Running "yarn dev" certainly works as expected.

[edwardtfn]

Same comment about /dist folder.

[avataar]

This one just packages it to dist/lovelace-sun-card.js and works too. Am I missing something?

[edwardtfn]

Are those , at the end of lines 28, 32 and 35 needed?

[avataar]

Either way is fine. For me personally having commas allows you to reorder things easily.