Skip to content

Update ISystemOperations.cs to give proper warning for MonitorEventsAsync#599

Open
MagnusMat wants to merge 3 commits into
dotnet:masterfrom
MagnusMat:patch-1
Open

Update ISystemOperations.cs to give proper warning for MonitorEventsAsync#599
MagnusMat wants to merge 3 commits into
dotnet:masterfrom
MagnusMat:patch-1

Conversation

@MagnusMat

Copy link
Copy Markdown

Changed Obselete warning to read out the correct shape of the MonitorEventsAsync Task.

Changed Obselete warning to read out the correct shape of the MonitorEventsAsync Task.
@MagnusMat MagnusMat changed the title Update ISystemOperations.cs Update ISystemOperations.cs to give proper warning for MonitorEventsAsync Nov 28, 2022

@HofmeisterAn HofmeisterAn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks. I guess it should be IProgress<Message> too.

Comment thread src/Docker.DotNet/Endpoints/ISystemOperations.cs Outdated
Co-authored-by: Andre Hofmeister <9199345+HofmeisterAn@users.noreply.github.com>
@HofmeisterAn

Copy link
Copy Markdown
Contributor

@galvesribeiro Can we merge this, please?

@galvesribeiro

Copy link
Copy Markdown
Member

The build is failing. Can you fix this first?

@HofmeisterAn

Copy link
Copy Markdown
Contributor

The build is failing. Can you fix this first?

I do not think the change and the failing build belongs together. The change only updates the description of an already obsolete method. There is another issue.

@dgvives

dgvives commented Dec 23, 2022

Copy link
Copy Markdown
Contributor

@HofmeisterAn from my experience with tests running on CI/CD pipeline, from time to time docker fails.
I suggest to push an empty commit to trigger the tests again, and they may work this time.

@HofmeisterAn

Copy link
Copy Markdown
Contributor

@HofmeisterAn from my experience with tests running on CI/CD pipeline, from time to time docker fails.
I suggest to push an empty commit to trigger the tests again, and they may work this time.

Yep, I agree. Although, an empty commit is not necessary IMO. The PR does not change anything expect documentation. Some tests are flaky, I noticed that too. I can take a look at that one.

@dgvives

dgvives commented Dec 28, 2022

Copy link
Copy Markdown
Contributor

@MagnusMat my suggestion about pushing a new empty commit to trigger the CI/CD pipeline was meant for you, I made a mistake when tagging the author

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