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

Updated Nokogiri version #78

Merged
merged 6 commits into from
Aug 17, 2022
Merged

Conversation

andsel
Copy link
Contributor

@andsel andsel commented Aug 16, 2022

Update Nokogiri dependency version

@andsel andsel requested a review from jsvd August 16, 2022 13:21
@andsel andsel removed the request for review from jsvd August 16, 2022 14:27
@andsel andsel force-pushed the fix/update_nokogiri branch from 49a04c5 to e5d426a Compare August 16, 2022 14:36
@andsel andsel requested a review from jsvd August 16, 2022 14:42
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

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

There's no point in leaving other versions to be tested in CI if we know they'll fail. So I suggest we introduce a new test matrix yml file that only does 8.4+ testing

@andsel
Copy link
Contributor Author

andsel commented Aug 16, 2022

@jsvd I've update the matrix, I've left the 8.x branch which currently resolves to 8.3.3 to avoid missing to re-enable it after 8.4 release, so that's expected to be 🔴 for a little while

@andsel andsel requested a review from jsvd August 16, 2022 16:35
@@ -0,0 +1,28 @@
_performance: &_performance
Copy link
Member

Choose a reason for hiding this comment

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

Instead of editing the matrix.yml you can just directly edit the travis.yml of this repo: https://github.com/logstash-plugins/logstash-filter-xml/pull/80/files.

You can see that the tests for that PR respect the build matrix from travis.yml of the PR itself: https://app.travis-ci.com/github/logstash-plugins/logstash-filter-xml/builds/254472445

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank's @jsvd for the hint, I fixed that and now the matrix build work as expected.

@andsel andsel requested a review from jsvd August 17, 2022 07:22
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: João Duarte <[email protected]>
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

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

LGTM

@andsel andsel merged commit e947ec5 into logstash-plugins:main Aug 17, 2022
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