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

Add open-epaper-link to latest #3003

Closed
wants to merge 1 commit into from

Conversation

DutchmanNL
Copy link
Contributor

No description provided.

@github-actions github-actions bot added auto-checked This PR was automatically checked for obvious criterias must be fixed The Adapter request got review/automatic feedback that is required to be fixed before another review labels Nov 26, 2023
@mcm1957 mcm1957 added the (RE-)REVIEW pending (mcm1957) Changes requested by review have been applied, re-review could be done. label Nov 27, 2023
@github-actions github-actions bot deleted a comment from mcm1957 Nov 27, 2023
@github-actions github-actions bot deleted a comment from mcm1957 Nov 27, 2023
@mcm1957
Copy link
Collaborator

mcm1957 commented Nov 27, 2023

First of all - THANK YOU for the time and effort you spend to maintain this adapter.

I would like to give some feedback based on my personal oppinion. This is NOT an offical review and @Apollon77 might have several additional suggestions or even a different oppinion to one or the other statement. Please feel free to contact him (or wait for a response from him) if you cannot follow my suggestions or want to discuss some special aspects.

  • errors reported by adapter checker must be fixed

    Please reduze size of logo

  • unused onStateChange handler

    You configred an onStateChange handler but it looks like this handler does not do any important tasks. Please remove the handler if the adapter does not react on state changes.

  • reevaluate state roles

    Only the values specified here (https://github.com/ioBroker/ioBroker.docs/blob/master/docs/en/dev/stateroles.md) may be used as state roles. Do not invent new names as this might disturb the functionalyity of other adapters or vis.

    In addition the roles MUST match the read/write functionality. As an example a role value.* requires that the state is a readable state. Input states (write only) should have roles like level.*. Please read the description carefully. States with role 'button' should (must) have attribute 'common.read=false' set. A button ( "Taster" in german only triggers when you press but else has no state), so it should also be not readable. This is also like HomeMatic does it. A switch has clear states true or false and so can be read.

    Please avoid using general roles (state, value) whnever a dedicated role exists.

  • src/lib/adapter-config.d.ts seems to be template

    Pleas check this file. interface with option1 and option2 seems to be result of creator template.

  • adapt testing to supported node releases

    So the recommended testmatrix is [16.x, 18.x, 20.x] or [18.x, 20.x] depending on engines requirments setting at package.json

    As node 16 is end-of-life now, it's ok to remove node 16 from testing. In this case please require node 18 as minimum node version at pacakge.json.

    Eventually consider to require node 18 for this new adapter already.

Thanks for reading and evaluating this suggestions.
McM1957

Please add a comment when you have reviewed and fixed the suggestionsor at least commented the suggestions and you think the adapter is ready for a re-review!

@mcm1957 mcm1957 removed the (RE-)REVIEW pending (mcm1957) Changes requested by review have been applied, re-review could be done. label Nov 27, 2023
@mcm1957 mcm1957 changed the title Add open-epaper-link.png Add open-epaper-link Nov 27, 2023
@mcm1957 mcm1957 changed the title Add open-epaper-link Add open-epaper-link to latest Nov 27, 2023
Copy link

Automated adapter checker

ioBroker.open-epaper-link

Downloads
NPM

  • ❗ [E142] logo is too big. It must be less or equal than 512x512
  • 👀 [W171] "common.title" is deprecated in io-package.json
  • 👀 [W400] Cannot find "open-epaper-link" in latest repository

Add comment "RE-CHECK!" to start check anew

@mcm1957
Copy link
Collaborator

mcm1957 commented Dec 1, 2023

@ticaki
@DutchmanNL
Did you notice the feedback? I do not see a comment thet the remarks have been processed and a re-review would be appropiate.

@DutchmanNL
Copy link
Contributor Author

@ticaki

@DutchmanNL

Did you notice the feedback? I do not see a comment thet the remarks have been processed and a re-review would be appropiate.

Hi, Urs seen it and Most of the already solves will Update the Review State as soon the last open item (onStateChange) is ready

@github-actions github-actions bot added the *📬 a new comment has been added label Dec 1, 2023
@mcm1957 mcm1957 removed the *📬 a new comment has been added label Dec 1, 2023
@mcm1957
Copy link
Collaborator

mcm1957 commented Dec 25, 2023

Any news?
reminder 01.01.2024

@mcm1957
Copy link
Collaborator

mcm1957 commented Jan 15, 2024

@DutchmanNL

How shall we continue?
Are you working in this?
Is someone else working on this adapter?

Please let me/us know if you encounter any problem and which timefram you expect to fix the mentionend issues.

mcm1957

reminder 31.01.2024

@mcm1957 mcm1957 added the stale PR seems has no activity, will be closed after some time label Jan 15, 2024
@mcm1957
Copy link
Collaborator

mcm1957 commented Jan 31, 2024

How should we continue?
Do you need any further help?

Please let me/us know (by adding a comment) when all issues noted earlier are processed by you and a new review seesm to be appropiate.

If there is no response until 28.2.2024 this PR might be closed.

reminder 15.2.2024

@mcm1957
Copy link
Collaborator

mcm1957 commented Feb 16, 2024

How should we continue?
Do you need any further help?

Please let me/us know (by adding a comment) when all issues noted earlier are processed by you and a new review seesm to be appropiate.

If there is no response until 28.2.2024 this PR might be closed.

reminder 22.2.2024

@mcm1957
Copy link
Collaborator

mcm1957 commented Feb 22, 2024

How should we continue?
Do you need any further help?

Please let me/us know (by adding a comment) when all issues noted earlier are processed by you and a new review seesm to be appropiate.

If there is no response until 28.2.2024 this PR will be closed.

reminder 28.2.2024

@mcm1957
Copy link
Collaborator

mcm1957 commented Feb 28, 2024

As there was no reaction to the lastest status polls this PR will be closed now.

Feel free to create a new PR at any time if you plan to further maintain / fix this adapter.

@mcm1957 mcm1957 closed this Feb 28, 2024
@mcm1957 mcm1957 added wontfix and removed 28.2.2024 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-checked This PR was automatically checked for obvious criterias must be fixed The Adapter request got review/automatic feedback that is required to be fixed before another review stale PR seems has no activity, will be closed after some time wontfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants