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

Auto Dashboard releases #183

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SimonFischer04
Copy link
Contributor

Fixes #118

@SimonFischer04
Copy link
Contributor Author

This should work after we get the change in the release-script. Now working on adding the github-action.

@DutchmanNL
Copy link
Contributor

DutchmanNL commented Nov 15, 2023

This should work after we get the change in the release-script. Now working on adding the github-action.

hmm, its failing for me at my local desk

Edit: solved by 76d2b15

> [email protected] dashboardRelease
> ts-node scripts/dashboard-release.mts

(node:68745) ExperimentalWarning: Importing JSON modules is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
file:///Users/nl69zh/Developer/community/opensource/ioBroker.esphome/scripts/dashboard-release.mts:42
    const response = await fetch('https://api.github.com/repos/esphome/esphome/releases')
                     ^
ReferenceError: fetch is not defined
    at getLatestDashboardVersion (file:///Users/nl69zh/Developer/community/opensource/ioBroker.esphome/scripts/dashboard-release.mts:42:22)
    at run (file:///Users/nl69zh/Developer/community/opensource/ioBroker.esphome/scripts/dashboard-release.mts:15:50)
    at file:///Users/nl69zh/Developer/community/opensource/ioBroker.esphome/scripts/dashboard-release.mts:47:7
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async loadESM (node:internal/process/esm_loader:91:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

@DutchmanNL
Copy link
Contributor

DutchmanNL commented Nov 15, 2023

This should work after we get the change in the release-script.

what change do we need? script itself runs great nice job!

edit: Understood, related to AlCalzone/release-script#154

@Apollon77 what do you think about the PR for release script?
automation itself for this adapter is very cool from @SimonFischer04 and runs smoothly https://github.com/DrozmotiX/ioBroker.esphome/actions/runs/6883725092/job/18724832355

@SimonFischer04
Copy link
Contributor Author

what change do we need? script itself runs great nice job!\n\nedit: Understood, related to AlCalzone/release-script#154

Yes and no. Only seems to run fine. Because when you look at the script you can see that the actuall running of the release script is commented out for now so that i can test the rest.
And you already found the release script pr..

If you arent in a rush to add this feature instantly i would for now jsut leave this as it is and wait a bit to see if upstream gets merged,

otherwiese i could temporarily add some overidde for the upstream compile changelog-plugin.

@DutchmanNL
Copy link
Contributor

If you arent in a rush to add this feature instantly i would for now jsut leave this as it is and wait a bit to see if upstream gets merged,

no rush I think the current concept and PR is the right way to go. Let's wait for feedback by maintainer of release script and follow-up.
Nice journey anyway, automated update/integration and suggestion for release script covered by an automated action.... like it

@SimonFischer04
Copy link
Contributor Author

hmm, its failing for me at my local desk\n\nEdit: solved by 76d2b15

I dont know if its necessary to introduce another dependency as node 16 is already EOL and its run only inside the github action anyway , so the end user of the adapter does not need node18.

@DutchmanNL
Copy link
Contributor

hmm, its failing for me at my local desk\n\nEdit: solved by 76d2b15

I dont know if its necessary to introduce another dependency as node 16 is already EOL and its run only inside the github action anyway , so the end user of the adapter does not need node18.

Only temporary so I am also able to run it locally until my it department finally deploys 18+ 😅

@SimonFischer04
Copy link
Contributor Author

SimonFischer04 commented Nov 16, 2023

Nice journey anyway, automated update/integration and suggestion for release script covered by an automated action.... like it

yeah I may kind of tend to "overengineer" my contributions - sometimes simple bugs led me to things that i think can be improved and then it may escalate a bit :D.
for the esphome adapter my original issue was that I updated to debian 12 => new python version => broke the dashboard. and then, well stuff happened... 😄 .
Also always trying to get fixes upstreamed as much as possible to help others (release script, actually also fixed typescript types for the client class in the nativeapi - dependency still needs to be updated here), ....

Had I similar journey a few months ago where another project had a dependency that updated which broke a template because the dependency changed from a default export to a named one.
So basically all I would have needed to do was change something like import func from 'dep' to import { func } from 'dep' in my code generated from the template. and well. ... I ended up fixing some stuff end re-writing the whole project generator to be in es-module format ...

@SimonFischer04
Copy link
Contributor Author

SimonFischer04 commented Nov 16, 2023

Only temporary so I am also able to run it locally until my it department finally deploys 18+ 😅

Well... Good that I have root / sudo access to all of my devices 😄

maybe just leave it in your test-branch and don't fully merge it in, so the "temporary" don't get quite permanent because we forget about it. also speaking of forgetting about it, after having it running once we hopefully don't have to touch it / run locally to test again and everything just works and updates the dashboard from time to time automagically 😄

@SimonFischer04
Copy link
Contributor Author

If you arent in a rush to add this feature instantly i would for now jsut leave this as it is and wait a bit to see if upstream gets merged,

no rush I think the current concept and PR is the right way to go. Let's wait for feedback by maintainer of release script and follow-up. Nice journey anyway, automated update/integration and suggestion for release script covered by an automated action.... like it

@DutchmanNL
Does the new implementation in the adapter directlly: #118 (comment) mean this is now useless?

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.

Dashboard Update
2 participants