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

refactor: store excluded references in a set which wraps IdentityHashMap #611

Merged
merged 2 commits into from
Nov 15, 2021
Merged

refactor: store excluded references in a set which wraps IdentityHashMap #611

merged 2 commits into from
Nov 15, 2021

Conversation

algomaster99
Copy link
Member

Reference #602

I thought I would refactor the code first as suggested by @slarse.

Just glancing at that code I would begin with wrapping that map in a set (with Collections.newSetFromMap), because the values aren't used :)

@algomaster99 algomaster99 changed the title refactor: store excluded references in a Set refactor: store excluded references in a set which wraps IdentityHashMap Nov 5, 2021
@algomaster99
Copy link
Member Author

I am not sure what went wrong here, because tests passed on my machine. @khaes-kth could you please re-run the workflow? I think the failure could be because of the flaky tests.

@slarse
Copy link
Collaborator

slarse commented Nov 6, 2021

@algomaster99 The scheduled builds on master have been failing for a few days, it's probably not related to this PR.

@khaes-kth
Copy link
Collaborator

@khaes-kth could you please re-run the workflow? I think the failure could be because of the flaky tests.

I did it, but it seems the problem isn't fix with a rerun.

@fermadeiral @monperrus : can you add @algomaster99 as a collaborator so that he can see the logs? It seems I do not have access to do it.

@monperrus
Copy link
Contributor

monperrus commented Nov 9, 2021 via email

@algomaster99
Copy link
Member Author

I did it, but it seems the problem isn't fix with a rerun.

The scheduled builds are failing as pointed out by Simon. I have created a PR to fix it, but it seems like that isn't the best way.

@algomaster99
Copy link
Member Author

The tests should pass here after #617 is merged.

@khaes-kth khaes-kth self-requested a review November 15, 2021 12:51
@algomaster99
Copy link
Member Author

@khaes-kth once all tests are green, you may proceed to merge. This PR just refactors the code and it doesn't fix anything.

@algomaster99
Copy link
Member Author

@khaes-kth ping for merge.

Copy link
Collaborator

@khaes-kth khaes-kth left a comment

Choose a reason for hiding this comment

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

LGTM!

@khaes-kth khaes-kth merged commit ad3c0dc into ASSERT-KTH:master Nov 15, 2021
@algomaster99 algomaster99 deleted the refactor-SelectiveForceImport branch November 15, 2021 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants