Skip to content

Conversation

@paulfantom
Copy link
Contributor

As in title.

Main changes:

  • setting default working directory per job
  • moving management of common env variables to env file
  • joining workflows together to reduce code duplication and simplify management
  • splitting long commands into multiple tasks so logs are easier to read
  • using golangci-lint action instead of manual steps
  • removing limiting from daily scheduled job (since code changes can be done only via PRs and manipulating default branch there is no reason to run those tasks daily. Daily job should be only running for components which depend on 3rd party that is out of our control)
  • added a grouping job for e2e tests

@paulfantom paulfantom requested a review from a team as a code owner January 19, 2022 13:27
@paulfantom
Copy link
Contributor Author

This would allow us to set up branch restriction rules.

@paulfantom
Copy link
Contributor Author

CI won't start as the workflow is completely rewritten and does not exist on master branch.

@cevian
Copy link
Collaborator

cevian commented Jan 25, 2022

@paulfantom
I see Invalid workflow file : .github/workflows/cli.yml#L124 The workflow is not valid. .github/workflows/cli.yml (Line: 124, Col: 9): Job 'tests-result' depends on unknown job 'test-end-to-end'. in the action run

Copy link
Collaborator

@cevian cevian left a comment

Choose a reason for hiding this comment

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

Overall looks good. a few comments

name: e2e results
if: always()
needs:
- test-end-to-end
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is the error for why the ci won't run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, this part is wrong and should be just - e2e. However, CI won't run for a different reason - it won't run because the workflow name changed and the new name is not present on master branch. This is some sort of DoS prevention mechanism on GitHub side.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This still needs to be fixed.

@paulfantom
Copy link
Contributor Author

Rebased and addressed code review comments.

Copy link
Collaborator

@cevian cevian left a comment

Choose a reason for hiding this comment

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

Looks good except that one issue with cli.yml that still needs fixing (name of the dependent test)

name: e2e results
if: always()
needs:
- test-end-to-end
Copy link
Collaborator

Choose a reason for hiding this comment

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

This still needs to be fixed.

@paulfantom paulfantom force-pushed the ci-refactor branch 14 times, most recently from 3caf768 to 3698c5a Compare February 15, 2022 11:34
@paulfantom paulfantom merged commit d78103f into timescale:master Feb 15, 2022
@paulfantom paulfantom deleted the ci-refactor branch February 15, 2022 12:32
This was referenced Feb 15, 2022
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.

2 participants