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

Add support to ignore nested properties #180

Merged
merged 4 commits into from
May 9, 2021
Merged

Add support to ignore nested properties #180

merged 4 commits into from
May 9, 2021
+125 −10

Conversation

TzeWey
Copy link
Contributor

@TzeWey TzeWey commented May 1, 2021

This PR addresses #132 with a unit test to verify the change

@TzeWey
Add support to ignore nested properties

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
156b3ef
mcollina
mcollina requested changes May 1, 2021
View reviewed changes
Copy link
Member

@mcollina mcollina 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! I've left a few comments

@TzeWey
Delete from deep clone of log object

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
6dcbe13
mcollina
mcollina reviewed May 2, 2021
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Can you add docs for this feature?

@coveralls
Copy link

Pull Request Test Coverage Report for Build 804081617

  • 0 of 13 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%
Totals Coverage Status
Change from base Build 797146268: 0.0%
Covered Lines: 276
Relevant Lines: 276
💛 - Coveralls

@coveralls
Copy link

coveralls commented May 2, 2021 •
edited
Loading

Pull Request Test Coverage Report for Build 823309577

  • 13 of 13 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%
Totals Coverage Status
Change from base Build 797146268: 0.0%
Covered Lines: 276
Relevant Lines: 276
💛 - Coveralls

@TzeWey
Update README

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
a972039
@TzeWey
Copy link
Contributor Author

TzeWey commented May 2, 2021

Updated README

mcollina
mcollina approved these changes May 2, 2021
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina requested a review from jsumners May 3, 2021 07:18
jsumners
jsumners requested changes May 4, 2021
View reviewed changes
@TzeWey
Update function description and add utils tests

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
e0e6b60
@TzeWey TzeWey requested a review from jsumners May 8, 2021 15:05
@jsumners jsumners merged commit 3477063 into pinojs:master May 9, 2021
@TzeWey TzeWey deleted the feature/nested-ignore branch May 9, 2021 18:34
@blaenk blaenk mentioned this pull request May 31, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina

@jsumners jsumners

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@TzeWey @coveralls @mcollina @jsumners