Begin Standardization and Rewrite #938

ecdye · 2020-06-01T00:02:38Z

Begins a rewrite of the openHABian core code to make it overall safer
and more verbose in failure. Additionally this helps to standardize the
coding format.

This contains major a major rewrite of 'packages.bash',
'nodejs-apps.bash', and other minor fixes in other files as well. This
also contains the proper testing functions using BATS for the rewritten
code.

Fixes #936
Related #937 #949
Fixes #856
Fixes #633

Signed-off-by: Ethan Dye [email protected]

functions/helpers.bash

CONTRIBUTING.md

holgerfriedrich

I have done a quick review of all files, overall looks good, thanks for this huge effort!

I did not look closely into the following parts:

Dockerfiles (you know better)
nodejs (not familiar with)
packages.bash (too many substantial changes to give a valid statement)
My recommendation would be to play around with all the installers in using packages.bash ... and if this is all fine for you, merge.

A few comments from my side, please check if you want to consider them.

holgerfriedrich · 2020-06-16T20:45:03Z

functions/helpers.bash

-
+  local repoKey
+
+  repoKey="$(mktemp "${TEMP:-/tmp}"/openhabian.XXXXX)"


never heard of $TEMP on Linux, just googled it
https://unix.stackexchange.com/questions/352107/generic-way-to-get-temp-path
propose to roll back to old command if there is no strong reason behind

Well we should have a variable a user can set.
Let's use $TMPDIR then.

Why that? /tmp is POSIX standard.... I prefer to keep it simple...

Its so that the user has the option to change the tmp dir as there were some complaints about it not being big enough in the community.

To be honest, I disagree. This is not the Linux way of dealing with that. Then tmp is too small anyway and needs to be mounted somewhere else.
My recommendation: Keep is simple and predictable. Don't do it.
@mstormi ?

Why that? /tmp is POSIX standard.... I prefer to keep it simple...

I believe you misunderstood. By default, /tmp IS used if $TMPDIR is unset, see code.
But for cases where it is too small - I myself have hit that on Java upgrade - the user must be able to override it, hence $TMPDIR.

This is not the Linux way of dealing with that.

Well, as a longterm admin I disagree. I've been using $TEMP 20 yrs ago already before Linux.
But this is not about Linux, let's shortcut dogma discussions .... as an openHABian maintainer trying to build a tool that saves users from a need to descend into UN*X hell, I see this to be a must. If there's whining about /tmp full, we can tell the user to set $TMPDIR to e.g. /var/tmp or some self-mounted temp dir.

holgerfriedrich · 2020-06-16T20:47:53Z

functions/helpers.bash

@@ -177,6 +184,12 @@ is_raspbian() {
  [[ "$(cat /etc/*release*)" =~ "Raspbian" ]]
  return $?
 }
+# TODO: Not official yet, update when os-release file actually reflects the name change
+is_raspios() {


As you mentioned above, this won't work yet.
A few days ago the 32 bit version still returned "Raspbian", the 64 bit returned "Debian"...

Anyhow, where should we try to distinguish? For now, a Raspbian has to be handled same as a Raspberry Pi OS. So why do we need an additional function?

Markus wanted it added to have the template there for when it is officially changed.

But it does neither work at the moment not does it have a good use case, so still my recommendation to remove it.

I'll take the pragmatic approach and leave it in (else this merge would take another day or two).
But we all know it's not correct yet, not used yet and needs to be revisited once we plan on making use of it.

functions/java-jre.bash

functions/nodejs-apps.bash

functions/packages.bash

ecdye · 2020-06-17T01:26:58Z

In all of my testing the functions I have changed have functioned properly. I am ready if you guys are.

Also this includes further changes in terms of the Standarization and Rewrite. Fixes openhab#949 Related openhab#519 openhab#625 Signed-off-by: Ethan Dye <[email protected]>

mstormi · 2020-06-17T15:04:48Z

thanks for your work!

mstormi · 2020-06-17T17:14:24Z

@ecdye please check the Adopt repo, many tests to fail, e.g.
https://travis-ci.com/github/mstormi/openhabian/jobs/350466738#L576

ecdye · 2020-06-17T17:18:06Z

I know, I am looking at it right now, I think that they updated some of the packages this morning and then screwed something up because up until now all tests have passed. I am working on figuring out if it is on our end that something needs to change or theirs. I'll post when I have updates.

ecdye · 2020-06-17T17:35:45Z

adoptium/infrastructure#1399 It's jfrog we are fine we just have to wait it out.

ecdye · 2020-06-18T18:56:20Z

The issue appears to have been fixed.

There was a minor error in the code for Node-RED installation following openhab#938. This corrects that error. Signed-off-by: Ethan Dye <[email protected]>

There was a minor error in the code for Node-RED installation following #938. This corrects that error. Signed-off-by: Ethan Dye <[email protected]>

This contains further rewrites that improve the consistency of the codebase. These were made in addition to the changes that were first started in openhab#938. Related openhab#937 Signed-off-by: Ethan Dye <[email protected]>

This contains further rewrites that improve the consistency of the codebase. These were made in addition to the changes that were first started in #938. Related #937 Signed-off-by: Ethan Dye <[email protected]>

ecdye force-pushed the fixes-2 branch 2 times, most recently from d53a15e to 71aaacb Compare June 1, 2020 17:40

ecdye changed the title ~~Begin Standardization and Re-Write~~ Begin Standardization and Rewrite Jun 1, 2020

ecdye force-pushed the fixes-2 branch 18 times, most recently from 5ad791d to 2abab28 Compare June 7, 2020 18:31

mstormi reviewed Jun 7, 2020

View reviewed changes

functions/helpers.bash Outdated Show resolved Hide resolved

ecdye force-pushed the fixes-2 branch 8 times, most recently from 3e7ea17 to 2264391 Compare June 8, 2020 17:29

mstormi approved these changes Jun 16, 2020

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

ecdye force-pushed the fixes-2 branch 3 times, most recently from 847d516 to d1173dd Compare June 16, 2020 20:33

holgerfriedrich reviewed Jun 16, 2020

View reviewed changes

ecdye force-pushed the fixes-2 branch from d1173dd to 1eb5e35 Compare June 17, 2020 01:26

ecdye force-pushed the fixes-2 branch 2 times, most recently from 7135167 to eaf4e46 Compare June 17, 2020 02:58

Documentation and coding guidelines update

ddcacba

Also this includes further changes in terms of the Standarization and Rewrite. Fixes openhab#949 Related openhab#519 openhab#625 Signed-off-by: Ethan Dye <[email protected]>

ecdye force-pushed the fixes-2 branch from eaf4e46 to ddcacba Compare June 17, 2020 03:14

mstormi mentioned this pull request Jun 17, 2020

Deprecate PINEA64, Jessie, Xenial #971

Merged

mstormi merged commit e2a5290 into openhab:master Jun 17, 2020

ecdye added this to the Refinement and Refocus milestone Jun 27, 2020

ecdye added a commit to ecdye/openhabian that referenced this pull request Jul 8, 2020

Fix broken code in Node-RED install

48cc456

There was a minor error in the code for Node-RED installation following openhab#938. This corrects that error. Signed-off-by: Ethan Dye <[email protected]>

ecdye mentioned this pull request Jul 8, 2020

Fix broken code in Node-RED install #1018

Merged

mstormi pushed a commit that referenced this pull request Jul 8, 2020

Fix broken code in Node-RED install (#1018)

10985d8

There was a minor error in the code for Node-RED installation following #938. This corrects that error. Signed-off-by: Ethan Dye <[email protected]>

ecdye mentioned this pull request Jul 17, 2020

Continue Standardization and Rewrite #990

Merged

ecdye deleted the fixes-2 branch August 2, 2020 03:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Begin Standardization and Rewrite #938

Begin Standardization and Rewrite #938

ecdye commented Jun 1, 2020 •

edited by mstormi

Loading

holgerfriedrich left a comment

holgerfriedrich Jun 16, 2020

mstormi Jun 16, 2020

holgerfriedrich Jun 16, 2020

ecdye Jun 17, 2020

holgerfriedrich Jun 17, 2020

mstormi Jun 17, 2020 •

edited

Loading

holgerfriedrich Jun 16, 2020

ecdye Jun 17, 2020

holgerfriedrich Jun 17, 2020

mstormi Jun 17, 2020 •

edited

Loading

ecdye commented Jun 17, 2020

mstormi commented Jun 17, 2020

mstormi commented Jun 17, 2020

ecdye commented Jun 17, 2020

ecdye commented Jun 17, 2020

ecdye commented Jun 18, 2020


		local repoKey

		repoKey="$(mktemp "${TEMP:-/tmp}"/openhabian.XXXXX)"

Begin Standardization and Rewrite #938

Begin Standardization and Rewrite #938

Conversation

ecdye commented Jun 1, 2020 • edited by mstormi Loading

holgerfriedrich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mstormi Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mstormi Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

ecdye commented Jun 17, 2020

mstormi commented Jun 17, 2020

mstormi commented Jun 17, 2020

ecdye commented Jun 17, 2020

ecdye commented Jun 17, 2020

ecdye commented Jun 18, 2020

ecdye commented Jun 1, 2020 •

edited by mstormi

Loading

mstormi Jun 17, 2020 •

edited

Loading

mstormi Jun 17, 2020 •

edited

Loading