Skip to content

Misc fixes for /question build#8881

Merged
GangWang01 merged 7 commits intodotnet:mainfrom
yuehuang010:main
Aug 4, 2023
Merged

Misc fixes for /question build#8881
GangWang01 merged 7 commits intodotnet:mainfrom
yuehuang010:main

Conversation

@yuehuang010
Copy link
Copy Markdown
Contributor

  • Fix an merge error in GenerateResource.NothingOutOfDate
  • Avoid the complex state in WriteLines when Lines are empty. /question just always returns true. Add a test.
  • Add Tracker to bootstrap, makes dogfooding C++ easier.
  • Add SkipUnchangedFiles to avoid extra copy.

* Add Question switch to stop the build when targets are not incremental.

* Add question property to tasks.

* Add tests

* Clean up IIncrementalTask interface.

* Add additional tasks.

* Test and Question on this repro.

* fix build

* Fix question in -m.  Fix BindingRedirect to target incremental.

* Fix tests for Linux.

* WIP

* Fix feedbacks

* .

* Revert condition.

* fix feedback.

* touch to rerun.

* Fix merge.

* Fix merge pt2

* Fix merge p3

* Fix fileState when it couldn't resolve some files.

* Fix merge

* Address feedbacks

* Fix test.

* Clean up.

* WIP

* Fix Feedback

* Fix Feedback.

* Update tests

* Address some feedbacks.

* Fix merge conflict

* .
@JanKrivanek
Copy link
Copy Markdown
Member

@yuehuang010 - would it be possible to split into separate PRs? I know it's small changes, but it's easier to review single intent.

  • The WriteLines change looks good to me.
  • Tracker possibly as well.
  • GenerateResource.NothingOutOfDate I'm possibly misunderstanding (NothingOutOfDate is now used for both states of condition - that feels strange) - explanation would help.
  • SkipUnchangedFiles - I need more details for that one.

@yuehuang010
Copy link
Copy Markdown
Contributor Author

yuehuang010 commented Jun 15, 2023

  • GenerateResource.NothingOutOfDate I'm possibly misunderstanding (NothingOutOfDate is now used for both states of condition - that feels strange) - explanation would help.

Ah, the message changed. I might need to add another message. The goal is to Log.Error then fail fast the task. Hopefully the Error can provide an starting point for investigation.

  • SkipUnchangedFiles - I need more details for that one.

This parameter will avoid copying the file if it determines if the source and destination are the same. It will avoid an unnecessary disk write.

@AR-May AR-May requested review from AR-May and JanKrivanek June 20, 2023 13:52
@yuehuang010 yuehuang010 reopened this Jun 21, 2023
Copy link
Copy Markdown
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.

The GenerateResource incrementality fail message should be more informative.

Other changes looks good to go to me!

@JanKrivanek
Copy link
Copy Markdown
Member

@yuehuang010 - your changes looks good and helpful! Do you want to spin off the GenerateResource incrementality into separate PR so that we can get this through signoff? (or improving it as part of this PR works great as well - if you'd agree with that).
Thank you!

@yuehuang010
Copy link
Copy Markdown
Contributor Author

Oh snap, I forgot about this PR. Lets quickly spin off GenRes.

Copy link
Copy Markdown
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!
Thank you!

Copy link
Copy Markdown
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

@GangWang01 GangWang01 merged commit e08f8d9 into dotnet:main Aug 4, 2023
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.

4 participants