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

Switch to a non-tuple based type tracking #16

Merged
merged 8 commits into from
Apr 16, 2020

Conversation

rw-access
Copy link
Contributor

Issues

Relates to #15 for Elasticsearch based type changes.

Details

Address some tech debt. Removed the brittle tuple-based type system and switched to something class-based. This simplifies the handling and will make future maintenance and type-based changes easier.

@rw-access rw-access requested a review from brokensound77 April 14, 2020 01:42
@rw-access rw-access changed the base branch from master to feature/backport April 14, 2020 15:18
@rw-access rw-access force-pushed the feature/backport-types-cleanup branch from 2112d92 to 7d65796 Compare April 14, 2020 16:19
@rw-access rw-access requested a review from aleksmaus April 16, 2020 14:00
Copy link
Contributor

@brokensound77 brokensound77 left a comment

Choose a reason for hiding this comment

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

Just a few small comments, otherwise LGTM 👍

This probably warrants doc updates/expansion for sure, especially with some small changes to the edge cases

CHANGELOG.md Outdated Show resolved Hide resolved
IDENT_RE = re.compile(r"^[_a-zA-Z][a-zA-Z0-9_]*$")


is_primitive = set(b.value for b in TypeHint.primitives()).__contains__
Copy link
Contributor

Choose a reason for hiding this comment

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

🎈

eql/types.py Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
@rw-access rw-access merged commit f3ca4ae into feature/backport Apr 16, 2020
@rw-access rw-access deleted the feature/backport-types-cleanup branch April 16, 2020 23:53
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