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

Implement CodePipelineEvent #247

Merged
merged 23 commits into from Sep 18, 2021
Merged

Conversation

whithajess
Copy link
Contributor

@whithajess whithajess commented Oct 29, 2019

Issue #, if available:

#246

Description of changes:

Implement CodePipeline Events from the official documentation:

Note: this includes code from #382 as it makes this cleaner.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-io
Copy link

codecov-io commented Oct 30, 2019

Codecov Report

Merging #247 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #247   +/-   ##
=======================================
  Coverage   74.59%   74.59%           
=======================================
  Files          20       20           
  Lines         681      681           
=======================================
  Hits          508      508           
  Misses        128      128           
  Partials       45       45

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d89e42d...f727954. Read the comment docs.


// CodePipelineEvent is documented at:
// https://docs.aws.amazon.com/AmazonCloudWatch/latest/events/EventTypes.html#codepipeline_event_type
type CodePipelineEvent struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest changing the name to CodePipelineCloudWatchEvent to disambiguate from CodePipelineJobEvent

Copy link
Collaborator

@bmoffatt bmoffatt left a comment

Choose a reason for hiding this comment

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

Change the name to CodePipelineCloudWatchEvent or similar to disambiguate from CodePipelineJobEvent

@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2021

Codecov Report

Merging #247 (f08396f) into main (d64324e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #247   +/-   ##
=======================================
  Coverage   71.63%   71.63%           
=======================================
  Files          19       19           
  Lines        1040     1040           
=======================================
  Hits          745      745           
  Misses        228      228           
  Partials       67       67           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d64324e...f08396f. Read the comment docs.

@whithajess
Copy link
Contributor Author

@bmoffatt should now meet what you were after.

@whithajess whithajess requested a review from bmoffatt July 21, 2021 23:22
events/codepipeline_cloudwatch.go Outdated Show resolved Hide resolved
events/codepipeline_cloudwatch.go Outdated Show resolved Hide resolved
events/codepipeline_cloudwatch.go Outdated Show resolved Hide resolved
type CodePipelineEventDetail struct {
Pipeline string `json:"pipeline"`

// From live testing this is always int64 not string as documented
Copy link
Collaborator

Choose a reason for hiding this comment

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

suspicious... example in docs appear inconsistent on this one across "Pipeline Execution State Change"/"Stage Execution State Change"/"Action Execution State Change"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ill just confirm this from real data as well as the one version left as a string on this line before I request re-review

@bmoffatt bmoffatt merged commit 250c0d5 into aws:main Sep 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants