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.

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

#161 tweak of dependencies #162

Merged

Conversation

thompson-tomo
Copy link
Contributor

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Fixes #161

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link

what-the-diff bot commented May 2, 2024

PR Summary

  • Integration of Microsoft SourceLink GitHub into Gridify.Elasticsearch
    This means that our project will now have better support for debugging as developers are able to step into the source code. It increases code quality, simplifies bug fixing and enhances user trust in our software.

  • Inclusion of System.Reflection.Emit into Gridify
    The addition of the System.Reflection.Emit package helps to modify the code during runtime. This upgrade improves execution performance, provides greater development possibilities, and the ability to dynamically create types, which overall contributes to the flexibility of our project.

@alirezanet
Copy link
Owner

Hi @thompson-tomo,
I'm not sure if this is a good alternative since you're removing a dependency in all dotnet versions and adding another one that also is dependent in other packages .
Am I missing something!?

@thompson-tomo
Copy link
Contributor Author

thompson-tomo commented May 2, 2024

the dependency that was added is a dev dependency and is not packaged with the release take look at the other projects for reference,
The reason we can remove the dependency is that it is included in all frameworks natively hence it is not needing to be added.

I am not sure why the tests are failing on this project but doubt they are related to my change.

Copy link
Owner

@alirezanet alirezanet left a comment

Choose a reason for hiding this comment

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

LGTM 👍💐, about the failing test I fixed it in another branch and it is not related to this PR.

@alirezanet alirezanet merged commit c11e32a into alirezanet:master May 7, 2024
1 of 3 checks passed
@thompson-tomo thompson-tomo deleted the chore/#161_DependencyWork branch May 7, 2024 20:14
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.

Cleanup dependencies
2 participants