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 #74

Merged
merged 49 commits into from
Feb 25, 2021
Merged

Refactor #74

merged 49 commits into from
Feb 25, 2021

Conversation

gmoro
Copy link
Collaborator

@gmoro gmoro commented Nov 24, 2020

Major refactor of the code base

Will use this as base for new scheme of branched and tagged releases from now on

Copy link
Member

@Vogtinator Vogtinator left a comment

Choose a reason for hiding this comment

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

Please run shellcheck and fix the reported errors. Indentation has to be fixed to be consistent.

jeos-firstboot-functions needs to be cleaned up, currently it's just a place where all kinds of stuff is in. Separating the "built-in" dialogs into module files might work.

Now there's a separate module hook needed for supporting jeos-config, but modules without that would still be shown in the list, but fail to run. Not sure what to do about that though.

files/usr/sbin/jeos-config Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/share/jeos-firstboot/jeos-firstboot-dialogs Outdated Show resolved Hide resolved
files/usr/share/jeos-firstboot/raspberrywifi Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
@gmoro gmoro requested a review from Vogtinator February 2, 2021 12:07
Copy link
Member

@Vogtinator Vogtinator left a comment

Choose a reason for hiding this comment

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

Reviewed again - no major differences. I should actually give this a test run sometime...

files/usr/share/jeos-firstboot/jeos-firstboot-dialogs Outdated Show resolved Hide resolved
Copy link
Member

@Vogtinator Vogtinator left a comment

Choose a reason for hiding this comment

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

Did some testing, found some issues.

files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/share/jeos-firstboot/modules/raspberrywifi Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Show resolved Hide resolved
@gmoro
Copy link
Collaborator Author

gmoro commented Feb 18, 2021

Anything else missing, would be good to merge that soon as there are other stuff to be merged.

Copy link
Member

@Vogtinator Vogtinator left a comment

Choose a reason for hiding this comment

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

Found a bug, will do a test run again

files/usr/share/jeos-firstboot/jeos-firstboot-functions Outdated Show resolved Hide resolved
files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/share/jeos-firstboot/jeos-firstboot-dialogs Outdated Show resolved Hide resolved
Copy link
Member

@Vogtinator Vogtinator left a comment

Choose a reason for hiding this comment

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

Gave it a test run

files/usr/sbin/jeos-config Outdated Show resolved Hide resolved
files/usr/share/jeos-firstboot/jeos-firstboot-functions Outdated Show resolved Hide resolved
Copy link
Member

@Vogtinator Vogtinator left a comment

Choose a reason for hiding this comment

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

There are some more instances of dialog which should be d instead, but that's something for later.

@gmoro gmoro merged commit de3a6b0 into master Feb 25, 2021
@Vogtinator Vogtinator deleted the refactor branch July 9, 2021 09:56
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.

None yet

2 participants