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

Ensure empty value from site_config are kept #280

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

j0k3r
Copy link
Owner

@j0k3r j0k3r commented Jan 7, 2022

For example, when user wants to replace a string with nothing, replace_string: will be used and shouldn't be skipped.
Also, adding more tests regarding previous PR.

For example, when user wants to replace a string with nothing, `replace_string:` will be used and shouldn't be skipped.
Also, adding more tests regarding previous PR.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 95.321% when pulling 85634a8 on fix/kep-empty-site-config-value into 526c18f on master.

@j0k3r j0k3r merged commit e1a7b3d into master Jan 7, 2022
@j0k3r j0k3r deleted the fix/kep-empty-site-config-value branch January 7, 2022 12:31
],
],
]);
$res = $graby->fetchContent('http://motherjones.com/politics/2012/02/mac-mcclelland-free-online-shipping-warehouses-labor');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we could store the response for mocking purposes in case the site ever gets down.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I started that for few cases but it's really boring to perform 😫
https://github.com/j0k3r/graby/tree/master/tests/fixtures/content

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I could try making an automatic rule for https://github.com/rectorphp/rector.

Copy link
Owner Author

Choose a reason for hiding this comment

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

You mean a rule that run the test suite and save content fetched in file?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Opened #282.

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.

3 participants