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

Replace deprecated lookupType and subDictPtr with get and findDict #176

Merged
merged 3 commits into from
Apr 29, 2021

Conversation

MakisH
Copy link
Member

@MakisH MakisH commented Apr 29, 2021

Closes #172.

This should work with all ESI versions since v1806 (update: since v1812). I want to check building this with that v1806 and maybe v1712 before merging.

Now, building with v2012 gives no warnings! 🎉

@davidscn mainly FYI, you may want to have a look at the diff.

@MakisH MakisH added this to the v1.0.0 milestone Apr 29, 2021
@MakisH MakisH requested a review from davidscn April 29, 2021 08:39
@MakisH MakisH self-assigned this Apr 29, 2021
@davidscn
Copy link
Member

@davidscn mainly FYI, you may want to have a look at the diff.

I want to have a look at the compiler output 😉 Very cool!

@MakisH MakisH marked this pull request as ready for review April 29, 2021 08:59
@MakisH
Copy link
Member Author

MakisH commented Apr 29, 2021

I checked with the following versions:

version .get<>() .findDict()
v1712
v1806 ✔️
v1812 ✔️ ✔️

Therefore, we should offer <version>_com1812-2012.tar.gz and <version>_com1706-1806.tar.gz (potentially 1706 can be 1606).

@davidscn
Copy link
Member

Ok, then the decision is up to you.If you still want to merge it we should document the compatibility somewhere.

@MakisH
Copy link
Member Author

MakisH commented Apr 29, 2021

Ok, then the decision is up to you.If you still want to merge it we should document the compatibility somewhere.

Good point, I updated the documentation page here.

@MakisH MakisH merged commit 10a8c4d into develop Apr 29, 2021
@MakisH MakisH deleted the fix-172 branch April 29, 2021 10:26
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.

2 participants