Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactore DependencyInjection #768

Merged
merged 35 commits into from
May 10, 2020
Merged

Refactore DependencyInjection #768

merged 35 commits into from
May 10, 2020

Conversation

daveMueller
Copy link
Collaborator

closes #728

@daveMueller
Merge pull request #1 from tonerdo/master
a6e0687 
update fork
@daveMueller
Merge pull request #2 from tonerdo/master
84c0aff 
Update fork
@daveMueller
Merge pull request #3 from tonerdo/master
5b4e316 
Update fork
@daveMueller
Merge pull request #4 from tonerdo/master
45075e1 
Update fork
@daveMueller
Merge pull request #5 from tonerdo/master
ef7d0b0 
Update fork
@daveMueller
Merge pull request #6 from tonerdo/master
a50f143 
Update fork
@daveMueller
Merge pull request #7 from tonerdo/master
28e038c 
Update fork
@daveMueller
Merge pull request #8 from tonerdo/master
a8198b6 
Update fork
@daveMueller
Merge pull request #9 from tonerdo/master
fce0e2d 
Update fork
@daveMueller
Merge pull request #10 from tonerdo/master
8b8505b 
Update fork
@daveMueller
Merge pull request #11 from tonerdo/master
566262b 
Update fork
@daveMueller
Merge pull request #12 from tonerdo/master
eb5969d 
Update fork
@daveMueller
Merge pull request #13 from tonerdo/master
a3f4a2f 
Update fork
@daveMueller
Merge pull request #14 from tonerdo/master
c6fb0fc 
Update fork
@daveMueller
Merge pull request #15 from tonerdo/master
903ccdf 
Update fork
@daveMueller
changes to remove DependencyInjection and DependencyInjectionExtensio…
51cf8bd 
…ns from msbuild tasks
@daveMueller
changes to remove DependencyInjection and DependencyInjectionExtensio…
85f08cd 
…ns from console
@daveMueller
changes to remove DependencyInjection and DependencyInjectionExtensio…
aa7519f 
…ns from collector
@daveMueller
adaptions to test
974ce5c
@daveMueller
remove DependencyInjection and DependencyInjectionExtensions
85e5bc8
@MarcoRossignoli MarcoRossignoli added the enhancement General enhancement request label Mar 20, 2020
@MarcoRossignoli
Copy link
Collaborator

I'll take a look asap thx!

@MarcoRossignoli
Copy link
Collaborator

We're close to next release, after that I'll review and merge this one.

MarcoRossignoli
MarcoRossignoli requested changes Mar 29, 2020
View reviewed changes
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli left a comment

Choose a reason for hiding this comment

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

Some nits, but generally looks good!
Thank's a lot!

src/coverlet.msbuild.tasks/Services.cs Outdated Show resolved Hide resolved
src/coverlet.msbuild.tasks/Services.cs Outdated Show resolved Hide resolved
@MarcoRossignoli
Copy link
Collaborator

@daveMueller I needed to do a fast update on code, can you resolve conflicts?You can skip collectors I took some of your code and merged.

@daveMueller
Merge pull request #17 from tonerdo/master
438132c 
Update fork
@daveMueller
Merge branch 'master' into 728_RefactoreDI
5a63ea8 
# Conflicts:
#	src/coverlet.collector/DataCollection/CoverageManager.cs
#	src/coverlet.collector/DataCollection/CoverageWrapper.cs
#	src/coverlet.collector/DataCollection/CoverletCoverageCollector.cs
#	src/coverlet.collector/Utilities/Interfaces/ICoverageWrapper.cs
#	src/coverlet.console/Program.cs
#	src/coverlet.msbuild.tasks/InstrumentationTask.cs
#	test/coverlet.collector.tests/CoverletCoverageDataCollectorTests.cs
@daveMueller
Copy link
Collaborator Author

@daveMueller I needed to do a fast update on code, can you resolve conflicts?You can skip collectors I took some of your code and merged.

No trouble at all. I resolved the conflicts.

MarcoRossignoli
MarcoRossignoli requested changes Apr 19, 2020
View reviewed changes
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli left a comment

Choose a reason for hiding this comment

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

some nits, thank's!

src/coverlet.collector/DataCollection/CoverageManager.cs Outdated Show resolved Hide resolved
src/coverlet.collector/DataCollection/CoverletCoverageCollector.cs Outdated Show resolved Hide resolved
src/coverlet.console/Program.cs Outdated Show resolved Hide resolved
MarcoRossignoli
MarcoRossignoli requested changes May 2, 2020
View reviewed changes
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli left a comment

Choose a reason for hiding this comment

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

Some nits but here we go!Thank's!

src/coverlet.collector/DataCollection/CoverageManager.cs Outdated Show resolved Hide resolved
src/coverlet.console/Program.cs Outdated Show resolved Hide resolved
@MarcoRossignoli
Copy link
Collaborator

You can resolve conflicts taking master files that project is already without statics.

@daveMueller
Merge pull request #18 from coverlet-coverage/master
54b7e29 
Update fork
@daveMueller
Merge branch 'master' into 728_RefactoreDI
ed1d3cb 
# Conflicts:
#	src/coverlet.collector/DataCollection/CoverageWrapper.cs
#	src/coverlet.collector/DataCollection/CoverletCoverageCollector.cs
@daveMueller
removed usings
54e58f6
@daveMueller
code review
1e5f1a5
MarcoRossignoli
MarcoRossignoli approved these changes May 4, 2020
View reviewed changes
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli left a comment

Choose a reason for hiding this comment

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

LGTM

@MarcoRossignoli
Copy link
Collaborator

/azp run

@MarcoRossignoli
Copy link
Collaborator

@tonerdo there is an issue with pipelines doesn't trigger anymore.

@MarcoRossignoli MarcoRossignoli added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label May 4, 2020
@daveMueller
Copy link
Collaborator Author

Any news on the problem that the pipline doesn't trigger anymore?

@MarcoRossignoli
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MarcoRossignoli MarcoRossignoli removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label May 10, 2020
@MarcoRossignoli
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MarcoRossignoli MarcoRossignoli merged commit 570cfc9 into coverlet-coverage:master May 10, 2020
@daveMueller daveMueller deleted the 728_RefactoreDI branch October 27, 2023 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarcoRossignoli MarcoRossignoli MarcoRossignoli approved these changes

Assignees
No one assigned
Labels
enhancement General enhancement request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactore DependencyInjection
2 participants
@daveMueller @MarcoRossignoli