Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

[#255] initDB set default to true #256

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

[#255] initDB set default to true #256

wants to merge 5 commits into from

Conversation

avasyukhin
Copy link

@avasyukhin avasyukhin commented Apr 17, 2019 •
edited by ppressives
Loading

Closes #255

@avasyukhin avasyukhin requested a review from ppressives April 17, 2019 12:06
@ppressives ppressives changed the title initDB set default to true [#255] initDB set default to true Apr 17, 2019
ppressives
ppressives reviewed Apr 17, 2019
View reviewed changes
core/src/main/scala/codesearch/core/Main.scala Outdated
@@ -16,7 +16,7 @@ object Main extends IOApp {
final case class Params(
updatePackages: Boolean = false,
downloadMeta: Boolean = false,
initDB: Boolean = false,
initDB: Boolean = true,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's not enough to consider this task completed. At least we should delete the make table task from code, Makefile, Readme, docker files, etc.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, i removed usages of -i flag

Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure that you removed all usages of this command. The most important place where it mentioned is docker-stack-compose.yml

@avasyukhin avasyukhin requested a review from ppressives April 17, 2019 13:44
ppressives
ppressives reviewed Apr 18, 2019
View reviewed changes
README.md Outdated
@@ -40,7 +38,7 @@ running. The port can be changed:
$ make serve port=7000

Note: if you get an error at the `make tables` stage, you probably haven't
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Note: if you get an error at the `make tables` stage, you probably haven't
Note: if you get an error at the `make serve` stage, you probably haven't

ppressives
ppressives reviewed Apr 18, 2019
View reviewed changes
README.md Outdated
@@ -30,8 +30,6 @@ You can run Postgres by yourself, but it's better to use Docker. If you have
Docker installed, you can do this:

$ make build # Build the project
$ make db # Download and start Postgres (wait a bit after this step)
Copy link
Contributor

Choose a reason for hiding this comment

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

We need this step, roll it back, please.

@avasyukhin avasyukhin requested a review from ppressives April 18, 2019 13:55
ppressives
ppressives previously approved these changes Apr 18, 2019
View reviewed changes
@kamilongus
Copy link
Contributor

@avasyukhin Please, update the README.MD file in ./docker directory too.
Also fix the following chapter: via docker-compose for use it via docker swarm.

@kamilongus kamilongus closed this Apr 18, 2019
@kamilongus kamilongus reopened this Apr 18, 2019
@avasyukhin
Copy link
Author

@avasyukhin Please, update the README.MD file in ./docker directory too.
Also fix the following chapter: via docker-compose for use it via docker swarm.
@kamilongus
I understand it right that I need to add swarm tasks analogical in effect to a current compose?
And are we planning to use Kubernetes in future?

@kamilongus
Copy link
Contributor

@avasyukhin

I understand it right that I need to add swarm tasks analogical in effect to a current compose? And are we planning to use Kubernetes in future?

Yes that's right. You can take a command to deploy from the circkeci config.yml config file and use it for actualize docker instructions in the README.MD file. Yes, we plan to use k8s in the future if our infrastructure requires it.

@avasyukhin
Copy link
Author

Ok, do we need to leave compose commands in readme or remove it?
I try to fix it on monday, but I think that changing compose to swarm must be a different issue from this.

@ppressives ppressives dismissed their stale review May 3, 2019 11:15

Task not finished

@avasyukhin avasyukhin requested a review from ppressives May 23, 2019 13:45
@kamilongus
Copy link
Contributor

@Kabowyad PR with issue now pinned to you

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ppressives ppressives Awaiting requested review from ppressives

Assignees

@Kabowyad Kabowyad

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove the make tables task and init db by default.
4 participants
@avasyukhin @kamilongus @ppressives @Kabowyad