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 to use one set of Docker instruction name constants when processing Docker instruction/command info #374

Open
kcq opened this issue Aug 27, 2022 · 7 comments · May be fixed by #545

Comments

@kcq
Copy link
Member

kcq commented Aug 27, 2022

No description provided.

@chirag0002
Copy link

Hey @kcq, I want to work on this, also I am very new to the project can you please drop me a few resources from where I can learn and contribute?
I have joined the community to get more involved.

@Omkar0114
Copy link
Contributor

Hi @kcq, @iximiuz go through this codebase you explained - https://www.youtube.com/watch?v=ioIG-oPg0pA
You took the session very nicely and I am going through the codebase.

The issue seems like when minifying the docker image need to provide a set of docker instructions/commands about images and containers similar to prompts?
Am I pointing right?
Please suggest the approach how should I go ahead with this
Thanks!

@kcq
Copy link
Member Author

kcq commented Jun 30, 2023

It's not limited to minifying images. There are several places in the code where the container instruction metadata is processed and those places use their own set of constants. For example:

https://github.com/slimtoolkit/slim/blob/master/pkg/docker/instruction/instruction.go#L9

https://github.com/slimtoolkit/slim/blob/master/pkg/docker/dockerfile/dockerfile.go#L19

https://github.com/slimtoolkit/slim/blob/master/pkg/docker/dockerfile/reverse/reverse.go#L107

@Omkar0114
Copy link
Contributor

Okay. Thanks for sharing the links Kyle. Can you share more details on the above code and what exactly needs to be done?
I'll quickly start working on this!

@Omkar0114
Copy link
Contributor

Omkar0114 commented Jul 1, 2023

Hey @kcq,
In this file - https://github.com/slimtoolkit/slim/blob/master/pkg/docker/instruction/instruction.go#L9

1st approach -

This can be done by creating new enum called DockerInstruction and adding all constants to it. For example -

enum DockerInstruction {
Add, 
Arg,
Cmd 
Copy,
};

And code in the spec map update to use constants from the enum. For example -

Specs["Add"] = Format{
	Name:             ADD,
	SupportsFlags:    true,
	SupportsJSONForm: true,
}

How's this approach IMO?

2nd approach -

we can create a new constant InstructionNames that maps all the supported instruction names to their corresponding string values

var InstructionNames = map[string]string{
	"add":         Add,
	"arg":         Arg,
	"cmd":         Cmd,
	"copy":        Copy,

@Omkar0114
Copy link
Contributor

Hey @kcq should I go ahead with this ?

@Omkar0114
Copy link
Contributor

Hi @kcq I am researching various approaches to meet the end goal.
Also can we create different docker_instructions.go and add all constants to it and refractor the code ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants