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

Update installer.sh #77

Closed
wants to merge 1 commit into from
Closed

Update installer.sh #77

wants to merge 1 commit into from

Conversation

firmianay
Copy link
Contributor

Seems no need to ask again

Seems no need to ask again
@p4cx
Copy link
Member

p4cx commented Mar 19, 2021

In my opinion, this is as completely correct as it was. The step with the extra query is filling the database and that takes a good while. The installer in its normal form, without the -f or so switches, is mainly intended for development. As a pure user you should simply use -f or -d for a Docker setup and not manually go through the installer.
@m-1-k-3 , what do you think of that?

@m-1-k-3
Copy link
Member

m-1-k-3 commented Mar 19, 2021

Thank you @firmianay to check our installer. Probably @firmianay will find something more that we can check on Monday? ;)

@m-1-k-3
Copy link
Member

m-1-k-3 commented Mar 22, 2021

@firmianay I have checked it out now and I can see your point. The cve-database should be installed in default as it is a main element of a lot of areas of emba. But, as we got the feedback to be more granular in the possibilities a user has we have implemented the current behavior. As the user gets the help output if he does not pass arguments to the installer it is fine for me.
The only thing I think we should change:
Move the default behavior to the first line of the help output and give a note that this should be the default usage for most user. Probably we should remove the “Force” and change it to “Install all dependencies – typical initial installation” or something like this. @p4cx are you fine with this?

@p4cx
Copy link
Member

p4cx commented Mar 22, 2021

The only thing I think we should change:
Move the default behavior to the first line of the help output and give a note that this should be the default usage for most user. Probably we should remove the “Force” and change it to “Install all dependencies – typical initial installation” or something like this.

That sounds fine. That's how we do it.

@firmianay
Copy link
Contributor Author

Thanks for reply. Providing a "Force" option is of course a good choice, I just feel that some dependencies are not clear, some should be a mandatory component, not optional. Another example is 'git', used in the front process, but installed later again.

@m-1-k-3
Copy link
Member

m-1-k-3 commented Mar 23, 2021

Interesting. Probably we need to review it again.

└─$ grep print_tool.*git installer.sh
print_tool_info "git" 1
print_tool_info "git" 1

@p4cx p4cx mentioned this pull request Mar 24, 2021
@p4cx
Copy link
Member

p4cx commented Mar 24, 2021

I will close this PR, the discussion for the review of the installer can now be found here: #85

Thanks to @firmianay again for your help.

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

3 participants