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

Refactored adapter, Node-RED 3.1.x, added nodes, fixed issues #407

Merged
merged 34 commits into from
Nov 26, 2023

Conversation

Copy link
Contributor

@Apollon77 Apollon77 left a comment

Choose a reason for hiding this comment

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

Thank you, I added some comments

README.md Show resolved Hide resolved
main.js Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@Apollon77 Apollon77 self-requested a review November 21, 2023 07:53
@Apollon77
Copy link
Contributor

@klein0r Cool many thanks for this huge update! Just tell when you are done then I do a final look over it and we can release that (minor or major WDYT?)

}
return `${line.substring(0, pos)}${setValue}${line.substring(pos + toFind.length)}`;

const cmd = `npm install ${npmLib} --omit=dev --prefix "${this.userDataDir}" --save`;

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input Medium

This string concatenation which depends on
library input
is later used in a
shell command
.
// cert: require("fs").readFileSync('cert.pem')
// }
// },
'%%secure%%'https: { key: require("fs").readFileSync("'%%certPrivate%%'"), cert: require("fs").readFileSync("'%%certPublic%%'") },

Check notice

Code scanning / CodeQL

Syntax error Note

Error: Unexpected token
const oldText = fs.existsSync(settingsPath) ? fs.readFileSync(settingsPath, 'utf8') : '';
const newText = lines.join('\n');
if (oldText !== newText) {
fs.writeFileSync(settingsPath, newText);

Check failure

Code scanning / CodeQL

Potential file system race condition High

The file may have changed since it
was checked
.
@klein0r
Copy link
Contributor Author

klein0r commented Nov 21, 2023

@Apollon77 I'm done so far. I will respond / check further issues if this is merged. Please release a major version, since I've changed a lot (e.g. admin config requires to re-enter all passwords). See added message in the io-package.json.

@klein0r
Copy link
Contributor Author

klein0r commented Nov 26, 2023

@Apollon77 ping before dependabot creates merge conflicts by end of month 😄

@Apollon77 Apollon77 merged commit 126c9aa into ioBroker:master Nov 26, 2023
13 of 14 checks passed
@Apollon77
Copy link
Contributor

5.0.0 on its way. Thank you very much!

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