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.

Sign up for GitHub

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

Jump to bottom

Modular parsing and code migration #13

Merged
merged 1 commit into from
Jan 28, 2021
Merged

Modular parsing and code migration #13

merged 1 commit into from
Jan 28, 2021

Conversation

mlr0p
Copy link
Contributor

@mlr0p mlr0p commented Jan 12, 2021 •
edited
Loading

This PR implements modular parsing to reduce code reuse, and also migrates the old code to the new template #11 .

@mlr0p mlr0p changed the title code migration Modular parsing and code migration Jan 12, 2021
@mlr0p mlr0p requested review from ss23 and SoftPoison January 12, 2021 23:13
SoftPoison
SoftPoison suggested changes Jan 13, 2021
View reviewed changes
Copy link

@SoftPoison SoftPoison left a comment

Choose a reason for hiding this comment

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

Good work overall, couple of small comments here and there but nothing major :)

importers/util/common.go Outdated Show resolved Hide resolved
importers/util/custom-parse.go Show resolved Hide resolved
importers/forbes.go Outdated Show resolved Hide resolved
importers/util/common.go Show resolved Hide resolved
@mlr0p mlr0p force-pushed the 11-generic-importer branch from 167c8f5 to 5834f08 Compare January 14, 2021 02:15
@mlr0p mlr0p requested a review from SoftPoison January 14, 2021 02:17
SoftPoison
SoftPoison approved these changes Jan 14, 2021
View reviewed changes
Copy link

@SoftPoison SoftPoison left a comment

Choose a reason for hiding this comment

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

LGTM

ss23
ss23 approved these changes Jan 27, 2021
View reviewed changes
Copy link
Contributor

@ss23 ss23 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mlr0p mlr0p merged commit 9540df6 into zxsecurity:master Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SoftPoison SoftPoison SoftPoison approved these changes

@ss23 ss23 ss23 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mlr0p @ss23 @SoftPoison