Pix4D / cogito

Concourse resource for GitHub Commit Status and Google Chat notifications
MIT License
35 stars 14 forks source link

Pci 2665 independent chat send add ability to specify sinks put params #108

Closed Esysc closed 1 year ago

Esysc commented 1 year ago

Part of: PCI-2665

This is the first part of the new feature, it's developed around put.params. The next PR will address the source resource configuration and it will complete the tests. Note also that the README will be updated accordingly in a separate PR.

What is this PR introducing?

Basically it adds a parameter sinks that can be specified in a put step. This new parameter is expected as a slice of strings and if nothing is specified it behaviors as before defaulting to all supported sinks. Currently only 2 are supported: github and gchat.

Example:

put step will only send to gchat, no Github status involved.

plan:
      - get: the-repo
        trigger: true
      - put: gchat
        params: 
         state: pending
         sinks: ["gchat"]
      ...

Best review commity by commit.

Todos

Esysc commented 1 year ago

Notes to reviewers:

marco-m-pix4d commented 1 year ago

Could you please update the PAT and rerun the CI for both PRs? We cannot merge with failing tests, and without passing tests we don't know what to review, the code could be proved wrong by the tests themselves. Merci.

Esysc commented 1 year ago

Could you please update the PAT and rerun the CI for both PRs? We cannot merge with failing tests, and without passing tests we don't know what to review, the code could be proved wrong by the tests themselves. Merci.

Done!

marco-m-pix4d commented 1 year ago

Hello @Esysc, could you please mention here (and in the other PR) when all the outstanding comments have been answered, so that I can have an overview look? (no pressure, take your time!). Merci :-)

Esysc commented 1 year ago

@Pix4D/integration , this PR is ready, I've addressed all the feedback I received until now. Note that it will be not merged as is, but after merging #109 that will add all the missing features and the test coverage.

Esysc commented 1 year ago

The CI is failing because of test commit SHA reached the max statuses, however this is fixed in the next PR that will be merge onto this branch. For this reason I'd like to keep as is showing that this PR is not intended to be merged until the other one will be.

marco-m-pix4d commented 1 year ago

Rebased on #110, this will fix the CI

marco-m-pix4d commented 1 year ago

The squash of all the commits into one reveal that this PR could have been simply the first commit of #109 :-)

Esysc commented 1 year ago

Closing, replaced by https://github.com/Pix4D/cogito/pull/113