Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
/ NuGet.Jobs Public archive

Status - add export logic for two-layer aggregation #566

Merged
merged 11 commits into from
Oct 9, 2018

Conversation

scottbommarito
Copy link

This PR fixes the export logic (that generates the status.json file) that was broken by #563.

Previously, only EventEntitys were exported--now we need to export IncidentGroupEntitys as well.

I would suggest starting at StatusExporter.

Copy link
Contributor

@agr agr left a comment

Choose a reason for hiding this comment

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

:shipit:

@joelverhagen
Copy link
Member

public class StatusContractResolver : DefaultContractResolver

Can we remove this? Seems like the specific JSON serialization is an implementation detail so simpler is better.


Refers to: src/StatusAggregator/Export/StatusContractResolver.cs:15 in e7c586e. [](commit_id = e7c586e, deletion_comment = False)


if (currentComponent == null)
{
_logger.LogWarning("Couldn't find component corresponding to active entities.");
Copy link
Member

Choose a reason for hiding this comment

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

LogWarning [](start = 36, length = 10)

Why isn't this an exception?

Copy link
Author

Choose a reason for hiding this comment

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

my bad, will fix

Copy link
Member

@joelverhagen joelverhagen left a comment

Choose a reason for hiding this comment

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

:shipit:

@scottbommarito scottbommarito merged commit c75c1c5 into sb-groupfactory Oct 9, 2018
@scottbommarito scottbommarito deleted the sb-groupexport branch October 9, 2018 19:09
joelverhagen pushed a commit that referenced this pull request Sep 27, 2019
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants