Skip to content

Conversation

whithajess
Copy link
Contributor

  • Incorrectly used "CodePipelineEvent"

Issue #, if available:

#244

Description of changes:

Moves CodePipelineJob into its own CodePipelineJobEvent giving back the CodePipelineEvent namespace for actual usage.

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

* Incorrectly used "CodePipelineEvent"
@codecov-io
Copy link

codecov-io commented Oct 28, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #245   +/-   ##
=======================================
  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...7980b8f. Read the comment docs.

@whithajess
Copy link
Contributor Author

@bmoffatt Did you have any thoughts on this? Thanks.

@bmoffatt
Copy link
Collaborator

@bmoffatt Did you have any thoughts on this? Thanks.

I'd rather not do a semver-major change. I'm ok with adding and alias type CodePipelineEvent = CodePipelineJobEvent to help with the ambiguity.

@whithajess
Copy link
Contributor Author

@bmoffatt I understand the reluctance for a semver-major but this will trip a lot of people up in the future even with the alias, it breaks the principle of least surprise all the other Events are what they say they are. This is what I did originally - Assumed that CodePipelineEvent was the same as CodeBuildEvent and then when digging in found that CodePipelineEvent was something completely unexpected!

@bmoffatt
Copy link
Collaborator

bmoffatt commented Dec 3, 2019

We won't be merging intentionally breaking changes to the event types at this time. I've tagged #244 as requiring a v2 release to keep track of the request.

For #247, a different type name can be used.

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.

3 participants