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

build: bump rules_docker and instructions for installing bazelisk #1680

Merged
merged 6 commits into from Jul 20, 2020

Conversation

subotic
Copy link
Collaborator

@subotic subotic commented Jul 17, 2020

No description provided.

@subotic subotic added build chore maintenance and build tasks labels Jul 17, 2020
@subotic subotic self-assigned this Jul 17, 2020
@subotic
Copy link
Collaborator Author

subotic commented Jul 17, 2020

@SepidehAlassi Ok, now it should really work :-)

@SepidehAlassi
Copy link
Contributor

SepidehAlassi commented Jul 20, 2020

It looks good with bazelisk, thanks for your work.
However, there is a problem with documentation:

`# build webapi

$ bazel build //webapi`

`#run api (webapi)

$ bazel run //:api`

Doing so one gets an error that the target 'api' is not declared in the package.
Screenshot 2020-07-20 08 33 07

I believe the correct target is webapi, so please either change the documentation to state
`#run api (webapi)

$ bazel run //:webapi`

Or change the configuration so that the target is api, then the documentation should be
`# build api

$ bazel build //api`

`#run api (api)

$ bazel run //:api`

@subotic
Copy link
Collaborator Author

subotic commented Jul 20, 2020

@SepidehAlassi I tried to clarify the installation instructions a bit more and I've fixed the other thing. Can you please approve?

$ bazel build //webapi

# run api (webapi)
$ bazel run //:api
Copy link
Contributor

Choose a reason for hiding this comment

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

@subotic so there is no bazel run command to run the api anymore ?
I remember there was a bazel run \\... to run everything, do you wanna add it to documentation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is. Just running the API does not make much sense without the whole stack. It won't work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With bazel you can only run one target at a time. This is why we still have make and docker-compose.

Copy link
Contributor

@SepidehAlassi SepidehAlassi left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

@subotic
Copy link
Collaborator Author

subotic commented Jul 20, 2020

@SepidehAlassi Thanks for the review :-)

@subotic subotic merged commit 55518d7 into develop Jul 20, 2020
@subotic subotic deleted the wip/DSP-494-bump_rules_docker branch July 20, 2020 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore maintenance and build tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants