Codecov Report

Merging #109 (4215389) into dev (41ed31a) will decrease coverage by 0.25%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #109      +/-   ##
==========================================
- Coverage   90.16%   89.91%   -0.25%     
==========================================
  Files          16       16              
  Lines         569      555      -14     
==========================================
- Hits          513      499      -14     
  Misses         56       56              

@nporter-adbe Can you investigate why the coverage dropped for this PR? It appears all your changes have 100% coverage, but I wonder if the changes to the tests are causing a code path to no longer get executed.

@kevinlind Yeah, that's interesting given the changes have 100% diff coverage. From what I can tell, it's caused by the fact I removed/simplified some code covered by testing, thus reducing the overall file size. Reducing the file size by removing tested code has resulted in the uncovered code to represent a larger portion of the file, which is NetworkResponseHandler.swift in this case.

@nporter-adbe Okay, that makes sense. It's odd that Codecov is reporting -17 lines because that appears to include lines removed and lines changed. The PR only removes about 6 lines.

I'm still debating whether it should be set to event source or type.

@shalehaha My take is that type has been used to signify which extension the event is coming from, even though this is a bit backward from how I'd expect source and type to be populated. So in this case I think we went with keeping that standard by having type set to com.adobe.edge.

Yes, the current way we define type/source always confuses me. But when I look at https://github.com/adobe/aepsdk-core-ios/blob/main/AEPCore/Sources/eventhub/EventType.swift, it more related to the feature/service, like analytics/target/rules. I think that is also what the type means in the edge respsone. But on the other side, you can also argue this event is from Edge service, so it should use edge as event type.
Another point is that it may be just less confusing if we use the type from edge as the type for sdk event, it is more consistent.
@nporter-adbe @emdobrin

@shalehaha Yeah, I can see both sides of the coin here, I wish we could revisit the definition of type/source for all events. My preference here is to keep as much consistency as possible and have com.adobe.edge be the type. It seems to not make much of a difference either way as long as we document them properly given that both options have valid reasons.

@shalehaha we discussed this originally for the Edge extension, we wanted to swap the type and source as it feels like it makes more sense that way, but we gave up to that idea for consistency across extensions. I think it shouldn't matter that much since we are using constants anyways when registering the listeners.