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.

Sign up for GitHub

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

Jump to bottom

docker: add .dockerignore, add build instructions #299

Merged
merged 1 commit into from
Aug 22, 2021
Merged

docker: add .dockerignore, add build instructions #299

merged 1 commit into from
Aug 22, 2021

Conversation

denysvitali
Copy link
Contributor

Description

The change adds an entry in the README on how to build the Docker image,
additionally a .dockerignore has been added.

Issues Resolved

None

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

mihirsoni
mihirsoni reviewed Apr 19, 2021
View reviewed changes
Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

@denysvitali Thanks for the PR. Just one small thought.

README.md Outdated
Comment on lines 61 to 67
## Building the Docker Image

```
yarn osd bootstrap
yarn build --docker
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you feel this be good place inside the Developer Guide ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, the Developer Guide seems to be more focused on code styling and development suggestions, while this may actually be used (as in my case) to build the project and deploy it somewhere :)

Copy link
Member

Choose a reason for hiding this comment

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

I can see an argument for putting it in the README. If we feel like it's not the place for it in the future we could potentially move it if we are unsure about placing it in the README @mihirsoni

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mind, my only points is we have many distribution. It could grow in future saying how to build .deb or .rpm. How about instead of making it docker specific, we make it generalized with yarn build --help and have a line below as just one example and communication for other build types explore yarn build --help command.

Thoughts @kavilla @denysvitali

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be an option, yes. But I would argue that nowadays Docker is the most portable solution and should be prioritized over the alternatives (rpms, debs). Usually, the first thing I look for a project (especially this size) is a Docker Image reference / Dockerfile

@odfe-release-bot
Copy link

✅   DCO Check Passed 8ac21fd

@kavilla
Copy link
Member

kavilla commented Apr 21, 2021 •
edited
Loading

@denysvitali thanks! Would it possible to create and link an issue for the improvement related to .dockerignore from this PR? It's pretty obvious but would be a nice to have for reference.

@denysvitali
Copy link
Contributor Author

@kavilla : sorry for the delay, done!

#389

@kavilla kavilla linked an issue May 30, 2021 that may be closed by this pull request
Add .dockerignore #389
Closed
kavilla
kavilla previously approved these changes Jul 13, 2021
View reviewed changes
tmarkley
tmarkley previously approved these changes Jul 28, 2021
View reviewed changes
Copy link
Contributor

@tmarkley tmarkley left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the change!

@tmarkley
Copy link
Contributor

Just realized that there is a conflict in the README after recent changes. I'll be adding a getting started section to the developer guide today, and we can put the Docker instructions there.

@tmarkley
Copy link
Contributor

tmarkley commented Aug 3, 2021

@denysvitali you can add the instructions to the developer guide now, changes are in main.

@denysvitali denysvitali dismissed stale reviews from tmarkley and kavilla via 3c56578 August 16, 2021 19:52
@denysvitali denysvitali force-pushed the feature/improve-docker branch from 8ac21fd to 3c56578 Compare August 16, 2021 19:52
@denysvitali
Copy link
Contributor Author

Done, sorry for the delay

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 3c56578

@kavilla kavilla self-requested a review August 17, 2021 04:32
@tmarkley tmarkley added v1.1.0 docs Improvements or additions to documentation labels Aug 22, 2021
@tmarkley tmarkley merged commit 8993e7c into opensearch-project:main Aug 22, 2021
tmarkley pushed a commit to tmarkley/OpenSearch-Dashboards that referenced this pull request Sep 1, 2021
@denysvitali
docs(docker): add .dockerignore, add build instructions (opensearch-p…
841fc3d 
…roject#299)

Signed-off-by: Denys Vitali <[email protected]>
@tmarkley tmarkley mentioned this pull request Sep 1, 2021
Merged
1 task
tmarkley pushed a commit that referenced this pull request Sep 1, 2021
docs(docker): add .dockerignore, add build instructions (#299) (#757)
a7d2118 
Signed-off-by: Denys Vitali <[email protected]>
@denysvitali denysvitali deleted the feature/improve-docker branch October 19, 2021 11:54
SuZhou-Joe pushed a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Mar 15, 2024
@raintygao
fix test for delete workspace modal (opensearch-project#299)
a92b40f 
Signed-off-by: tygao <[email protected]>
SuZhou-Joe pushed a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Mar 18, 2024
@raintygao @SuZhou-Joe
fix test for delete workspace modal (opensearch-project#299)
f95cf18 
Signed-off-by: tygao <[email protected]>
raintygao added a commit to raintygao/OpenSearch-Dashboards that referenced this pull request Mar 18, 2024
@raintygao
fix test for delete workspace modal (opensearch-project#299)
4b12d65 
Signed-off-by: tygao <[email protected]>
raintygao added a commit to raintygao/OpenSearch-Dashboards that referenced this pull request Mar 21, 2024
@raintygao
fix test for delete workspace modal (opensearch-project#299)
50ebda4 
Signed-off-by: tygao <[email protected]>
raintygao added a commit to raintygao/OpenSearch-Dashboards that referenced this pull request Mar 21, 2024
@raintygao
fix test for delete workspace modal (opensearch-project#299)
6e68a53 
Signed-off-by: tygao <[email protected]>
raintygao added a commit to raintygao/OpenSearch-Dashboards that referenced this pull request Mar 25, 2024
@raintygao
fix test for delete workspace modal (opensearch-project#299)
92156c1 
Signed-off-by: tygao <[email protected]>
raintygao added a commit to raintygao/OpenSearch-Dashboards that referenced this pull request Mar 26, 2024
@raintygao
fix test for delete workspace modal (opensearch-project#299)
7a735eb 
Signed-off-by: tygao <[email protected]>
ZilongX pushed a commit that referenced this pull request Mar 26, 2024
@raintygao
[Workspace] Add workspace list page (#6182)
2a94f32 
* feat: add workspace list

Signed-off-by: tygao <[email protected]>

* doc: update changelog

Signed-off-by: tygao <[email protected]>

* fix test for delete workspace modal (#299)

Signed-off-by: tygao <[email protected]>

* update function name and modal

Signed-off-by: tygao <[email protected]>

---------

Signed-off-by: tygao <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 18, 2024
@github-actions
[Workspace] Add workspace list page (#6182)
e4d78d0 
* feat: add workspace list

Signed-off-by: tygao <[email protected]>

* doc: update changelog

Signed-off-by: tygao <[email protected]>

* fix test for delete workspace modal (#299)

Signed-off-by: tygao <[email protected]>

* update function name and modal

Signed-off-by: tygao <[email protected]>

---------

Signed-off-by: tygao <[email protected]>
(cherry picked from commit 2a94f32)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
SuZhou-Joe pushed a commit that referenced this pull request Apr 18, 2024
@opensearch-trigger-bot @github-actions
[Workspace] Add workspace list page (#6182) (#6525)
157f6db 
* feat: add workspace list

Signed-off-by: tygao <[email protected]>

* doc: update changelog

Signed-off-by: tygao <[email protected]>

* fix test for delete workspace modal (#299)

Signed-off-by: tygao <[email protected]>

* update function name and modal

Signed-off-by: tygao <[email protected]>

---------

Signed-off-by: tygao <[email protected]>
(cherry picked from commit 2a94f32)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mihirsoni mihirsoni mihirsoni left review comments

@kavilla kavilla kavilla approved these changes

@tmarkley tmarkley tmarkley approved these changes

Assignees
No one assigned
Labels
docs Improvements or additions to documentation v1.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add .dockerignore
6 participants
@denysvitali @odfe-release-bot @kavilla @tmarkley @opensearch-ci-bot @mihirsoni