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

Use HashSet from BCL instead of custom implementation. #92

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Giorgi
Copy link

@Giorgi Giorgi commented Dec 11, 2015

Use generic System.Collections.Generic.HashSet from BCL instead of implementing it manually.

@martinwoodward
Copy link
Member

Thanks for the PR @Giorgi! It may take us a while to get to this one (see the Historical Note section in Scott's blog) because we're heads down focusing on the Blogger API fix (see #63 and #5) and then some other crashes and high priority bugs we're seeing such as #26 etc. There sure is a lot of code improvements that could be made though once we have a stable release that is working for the majority of folks so thanks for taking a look.

@Giorgi
Copy link
Author

Giorgi commented Dec 12, 2015

@martinwoodward I agree, shipping feature is more important. Do you think we need a new tag for PR-s that are refactoring and/or getting rid of legacy code so that we can easily track such issues?

Having a readable code is important too because it makes contributing easier. Most of the changes in this PR are just adding new using statement and a generic type parameter.

@kathweaver
Copy link
Contributor

From @ScottIsAFool on another one: We have moved to the dotnetfoundation's AppVeyor account and ScottH has closed his account which is what we used to be using. If @vhanla were to make an arbitrary commit to this branch and push it, it would force a rebuild on appveyor.

I'll test once it's updated.

@RobDolinMS
Copy link
Contributor

@Giorgi BIG THANK YOU for submitting this PR.
As we moved to a new AppVeyor account, would you mind making a minor update (to trigger a new build)
THANK YOU again for the PR and sorry for the long-delayed review.

@ScottIsAFool
Copy link
Member

Hi @Giorgi, it looks like there are conflicts at the moment that need to be resolved in order for this pull request to be built, could you please resolve these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants