Skip to content
This repository has been archived by the owner on Aug 6, 2023. It is now read-only.

Build database from intercept pointers declaration #28

Closed
wants to merge 4 commits into from
Closed

Build database from intercept pointers declaration #28

wants to merge 4 commits into from

Conversation

kymckay
Copy link
Contributor

@kymckay kymckay commented Dec 18, 2017

Reads the intercept pointers declaration file to generate the database.py file.

Closes #25

@kymckay
Copy link
Contributor Author

kymckay commented Dec 18, 2017

Just noticed a slight issue in that operators aren't included (==, !, etc.)

Future updates will just have to remember to re-add these (shouldn't be
an issue since the git diff will be more clean)
@kymckay
Copy link
Contributor Author

kymckay commented Dec 18, 2017

Fixed that, the issue now is that the unit tests are case sensitive against the error message which contains the operator names. Unfortunately the operators in the parsed file are all lowercase.

One solution would be to edit the unit tests to ignore case (or I suppose it would be better to convert the operator names to lowercase in the assertEqual calls). It would be less pretty, but not the end of the world.

@jonpas
Copy link
Contributor

jonpas commented Dec 19, 2017

Couldn't you just hard-code those in the generation script?

@kymckay
Copy link
Contributor Author

kymckay commented Dec 19, 2017

The symbols? Yeah not a bad idea

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.688% when pulling dbd71ea on SilentSpike:master into 04075b1 on LordGolias:master.

@LordGolias
Copy link
Owner

Merged in 3820b60.

I did some minor changes to clean PEP8 and make it a single commit, and merged it to master.

Thanks a lot @SilentSpike for this commit, much better than manually doing the work from intercept side.

@LordGolias LordGolias closed this Dec 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants