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

Refactor JavaScript to TypeScript #81

Merged
merged 2 commits into from
Nov 22, 2023
Merged

Conversation

DigiLive
Copy link
Collaborator

I've refactored the JavaScript files to TypeScript.

TypeScript is JavaScript with syntax for types and also used by Home Assistant.
It allows developers to precisely define the expected shape of objects and function parameters, resulting in code that's easier to understand and maintain.

Please test this code very carefully, so we're sure it matches the JavaScript version.

@@ -0,0 +1,262 @@
import {HomeAssistant} from "custom-card-helpers";

export namespace generic {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there is a risk overusing namespaces?
Will prefixing all these types with generic make the code a little less concise and readable?
My experience says namespaces are used in huge codebases where identical names of types needs to be separated. I am not convinced namespaces are really needed in this codebase though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just for now it's name-spaced to avoid such conflicts and for my own recognition.
It's a good comment though. The ones I need to optimize the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

So it is a definite no on removing namespace? 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope. I haven't gotten around to it yet and I really like to merge this PR since it disrupts other PRs.

@DigiLive DigiLive self-assigned this Oct 31, 2023
src/Helper.ts Outdated Show resolved Hide resolved
src/Helper.ts Outdated Show resolved Hide resolved
src/Helper.ts Outdated Show resolved Hide resolved
src/Helper.ts Outdated Show resolved Hide resolved
src/Helper.ts Outdated Show resolved Hide resolved
src/Helper.ts Outdated Show resolved Hide resolved
src/Helper.ts Outdated Show resolved Hide resolved
src/Helper.ts Outdated Show resolved Hide resolved
src/types/cards.ts Outdated Show resolved Hide resolved
src/types/generic.ts Outdated Show resolved Hide resolved
src/chips/AbstractChip.ts Outdated Show resolved Hide resolved
@DigiLive DigiLive mentioned this pull request Nov 13, 2023
@DigiLive DigiLive marked this pull request as ready for review November 13, 2023 07:51
@@ -0,0 +1,262 @@
import {HomeAssistant} from "custom-card-helpers";

export namespace generic {
Copy link
Contributor

Choose a reason for hiding this comment

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

So it is a definite no on removing namespace? 😄

src/cards/AbstractCard.ts Outdated Show resolved Hide resolved
src/cards/AbstractCard.ts Outdated Show resolved Hide resolved
src/cards/BinarySensorCard.ts Show resolved Hide resolved
@DigiLive DigiLive linked an issue Nov 13, 2023 that may be closed by this pull request
src/views/HomeView.ts Show resolved Hide resolved
src/cards/LightCard.ts Show resolved Hide resolved
@DigiLive DigiLive requested a review from AalianKhan November 17, 2023 15:01
@AalianKhan
Copy link
Owner

All of the other options use snake case, ex card_options. I would prefer if homeView would be changed to home_view.

@DigiLive
Copy link
Collaborator Author

@AalianKhan Since merging this PR into main is too complex, I'll have to alter the branches locally. This will probably change the history of the main branch.

If so, I'll need to force push my local main branch to origin.
Do you have any protection rules set which will prevent me from doing so?

Warning

If I change the history of the main branch, all current PRs must refetch this branch and possibly rebase their feature branch onto main.

@AalianKhan
Copy link
Owner

I don't believe so.
I have squashed and merged before without any issue. Maybe you are a collaborator so something might come up. If so I can go ahead and merge it.

Other changes include:
* Add Vacuum Card and View.

* Add hiding sections from the Home view by strategy configuration.

* Add hiding a view if no device is configured for it. Closes #24.

* Add color temperature to light card.

* Add icon to HomeView configuration.

* Add version output to console.

* Add version bumper.

* Add .editorconfig settings.

* Refactor README, because the information moved to the repository's
  Wiki. Closes #87.
@DigiLive DigiLive force-pushed the javascript-to-typescript branch from b6cbc79 to 12e4d76 Compare November 22, 2023 19:34
@DigiLive DigiLive merged commit 82a91c3 into main Nov 22, 2023
@DigiLive DigiLive added the enhancement New feature or request label Nov 22, 2023
@DigiLive DigiLive deleted the javascript-to-typescript branch February 3, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants