Skip to content
This repository has been archived by the owner on Jul 15, 2022. It is now read-only.

PRJ-778 Optimize Dockerfile and its workaround #17

Closed
wants to merge 18 commits into from

Conversation

nitrotol
Copy link
Contributor

Resolve PRJ-778

Optimize Dockerfile and its workaround to almost perfect base image

Copy link
Member

@Avvessalom Avvessalom 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 for contributing! A quick note on the code style

Comment on lines +25 to +27
function string_filter {
sed -z 's/ */ /g;s/\n//g' $1
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function string_filter {
sed -z 's/ */ /g;s/\n//g' $1
}
string_filter() {
sed -z 's/ */ /g;s/\n//g' $1
}

Comment on lines +62 to +82
*datagrip*)
SOFTWARE_LIST=$(string_filter <<- EOM

EOM
)
ADDITIONAL_COMMANDS=$(string_filter <<- EOM

EOM
);;

#
# Block for PhpStorm
*PhpStorm*)
SOFTWARE_LIST=$(string_filter <<- EOM

EOM
)
ADDITIONAL_COMMANDS=$(string_filter <<- EOM

EOM
);;
Copy link
Member

Choose a reason for hiding this comment

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

A lot of duplication, and I have no idea how to make it more compact. If you have, please do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have some ideas about this. In few days I`ll try to add smth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Avvessalom unfortunately I cant optimize this(

For now I have only one solution without code duplcation, but it required a many (at least one for each variant of IDE) files, witch store software list for each IDE.
But I think this is not a good solution.

Maybe we can accept this solution and, in future, give more convinient way?

Copy link
Member

Choose a reason for hiding this comment

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

Discussed here: #16 (comment). So let's leave it if we can't improve it more.

@SerVB
Copy link
Member

SerVB commented Dec 2, 2021

Could you please clarify why there are two PRs (this one and #16) with some colliding changes? Which one you want to merge?

Please also squash some commits it the both PRs: for now, there are many.

@yole
Copy link
Contributor

yole commented Jul 15, 2022

We're discontinuing the development of projector-docker and no longer accepting contributions, sorry.

@yole yole closed this Jul 15, 2022
@cqupt-zjs
Copy link

cqupt-zjs commented Jul 15, 2022 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants