Skip to content

Conversation

@syntrust
Copy link

@syntrust syntrust commented Dec 18, 2024

Error log:

ERROR: process "/bin/sh -c cd da-server && go build -o da-server main.go" did not complete successfully: exit code: 1

The error happens because there is no code to build, so we update the submodules before build.

@qzhodl
Copy link

qzhodl commented Dec 18, 2024

I’m curious why the tests passed in the PR without cloning the code. Could you clarify?

@syntrust
Copy link
Author

I’m curious why the tests passed in the PR without cloning the code. Could you clarify?

From what I can figure out, the PR is tested using the Makefile but not with CircleCI.
The Makefile defines the dependency path as devnet-up: pre-devnet: submodules. Heresubmodules pulls code from the da-server using the command git submodule update --init.

However, in .circleci/config.yml, this command is only present in the install-contracts-dependencies step used by contract-related jobs.
But *-docker-build jobs like op-node-docker-build in this issue build go code in Docker containers and have no submodule introduced before da-server.

@qzhodl
Copy link

qzhodl commented Dec 18, 2024

I’m curious why the tests passed in the PR without cloning the code. Could you clarify?

From what I can figure out, the PR is tested using the Makefile but not with CircleCI. The Makefile defines the dependency path as devnet-up: pre-devnet: submodules. Heresubmodules pulls code from the da-server using the command git submodule update --init.

However, in .circleci/config.yml, this command is only present in the install-contracts-dependencies step used by contract-related jobs. But *-docker-build jobs like op-node-docker-build in this issue build go code in Docker containers and have no submodule introduced before da-server.

Got it. I would suggest re-running the tests involved in the PR to ensure that this change has no unintended side effects.

@syntrust
Copy link
Author

I’m curious why the tests passed in the PR without cloning the code. Could you clarify?

From what I can figure out, the PR is tested using the Makefile but not with CircleCI. The Makefile defines the dependency path as devnet-up: pre-devnet: submodules. Heresubmodules pulls code from the da-server using the command git submodule update --init.
However, in .circleci/config.yml, this command is only present in the install-contracts-dependencies step used by contract-related jobs. But *-docker-build jobs like op-node-docker-build in this issue build go code in Docker containers and have no submodule introduced before da-server.

Got it. I would suggest re-running the tests involved in the PR to ensure that this change has no unintended side effects.

In order to avoid the possible side effects and keep the impact in the CircleCI scope, I would suggest this change instead. It is also future-compatible for possible new submodules. Idea?

@qzhodl
Copy link

qzhodl commented Dec 18, 2024

I’m curious why the tests passed in the PR without cloning the code. Could you clarify?

From what I can figure out, the PR is tested using the Makefile but not with CircleCI. The Makefile defines the dependency path as devnet-up: pre-devnet: submodules. Heresubmodules pulls code from the da-server using the command git submodule update --init.
However, in .circleci/config.yml, this command is only present in the install-contracts-dependencies step used by contract-related jobs. But *-docker-build jobs like op-node-docker-build in this issue build go code in Docker containers and have no submodule introduced before da-server.

Got it. I would suggest re-running the tests involved in the PR to ensure that this change has no unintended side effects.

In order to avoid the possible side effects and keep the impact in the CircleCI scope, I would suggest this change instead. It is also future-compatible for possible new submodules. Idea?

LGTM

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants