Skip to content

Conversation

@maridematte
Copy link
Member

Fixes #10145

Context

It is currently not possible to check for global variables during build execution. To have access to that data it needs to be passed on events to the processing nodes. None of the events we had previously were good matches to pass along this information, so it was decided to create one for build submission started, as this point in the build we have all properties loaded with the correct value, but its not too late to make use of them.

Changes Made

  • Added a new BuildSubmissionStartedEventArgs based on BuildStatusEventArgs and added it to the event handlers.
  • A copy of the enum BuildRequestDataFlags was added to Microsoft.Build.Framework.

Testing

To be added.

Notes

Blocker for #9747

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

This looks good as an initial draft. It needs couple finalization steps:

  • de/serialization of the event for node to node communication.
  • de/serialization of the event for BinaryLogger (plus version increase)
  • unit tests - for the de/serialization
  • fixing the build failures

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Looks good.

There are couple adjustments needed (versioning, nullable strings ...) - captured in comments

@maridematte maridematte requested a review from JanKrivanek August 7, 2024 06:46
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Looks good!

Please add the created issue into the sprint - so that it gets handled soon

Copy link
Member

@AR-May AR-May left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Add BuildSubmissionStartedEventArgs event

4 participants