-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add custom volumes as an option #4401
base: master
Are you sure you want to change the base?
Conversation
Hey @jdrouet can you take a look at this PR? |
Is this feature forgotten? |
@Marahin I'm ready here to provide any updates to get this into kitematic. Unfortunately the contributors are showing bad faith by simply ignoring it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't check the feature by itself yet but there are some changes to make in your PR ;)
} else { | ||
icon = <a onClick={this.handleRemoveVolume.bind(this, index)} className="only-icon btn btn-action small"><span | ||
className="icon icon-delete"></span></a>; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably take this piece of code out of this function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm not sure where to place it. Could I leave this one up to you? I was following the same code style as the other ContainerSetting pages:
kitematic/src/components/ContainerSettingsGeneral.react.js
Lines 217 to 221 in 1b68613
if (index === this.state.env.length - 1) { | |
icon = <a onClick={this.handleAddEnvVar} className="only-icon btn btn-positive small"><span className="icon icon-add"></span></a>; | |
} else { | |
icon = <a onClick={this.handleRemoveEnvVar.bind(this, index)} className="only-icon btn btn-action small"><span className="icon icon-delete"></span></a>; | |
} |
I just wanted to drop-in and say thank you @jdrouet for finding time to check this out, it's something I'm looking forward to see in Kitematic. 👍 |
Thank you @jdrouet! |
Hey, is there any progress on reviewing this @jdrouet ? How can we help? cc @stereokai |
Ship it! 🚀Please 👍 |
come on, we need this feature in Kitematic |
@jdrouet so what's happening with this PR? |
🙌 |
🎉 |
@aaomidi Your PR has been approved |
Nothing on my side unfortunately. |
Merges Pull Request: docker#3462 .. docker#3692 .. docker#4401 .. docker#5306 They are Fix Login. and adding stuff. temporarily simplifying the building. and remove the test.
Merges Pull Request: docker#3462 .. docker#3692 .. docker#4401 .. docker#5306 They are Fix Login. and adding stuff. temporarily simplifying the building. and remove the test. add building on TC with tag. and pushing release to github.
What can be done to get this merged? What are the blockers? |
https://github.com/DockStation/dockstation fits the bill. |
Bump - pls merge |
What happened on releasing? It has been more than 1 year since the PR pushed ! |
So slow :/ |
If there is interest I'll push a release of the binaries to my fork... |
I instinctively checked your repo for releases but https://github.com/DockStation/dockstation was a nice workaround. With this slow merging I would say everyone should ignore kitematic and support dockstation instead. |
Yeah I don't think I want to be responsible for maintaining a fork of this :D |
#1923 #4399
These issues were closed with no resolution. I decided to go ahead and make these changes.
The code changes how the volume section is handled to be the same as the environment variables. Allow custom volumes and deleting of any existing volumes.
This PR can be further extended to ensure that if a volume change broke Docker, it can revert back to a cached list of mounts.