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

ENT-5850: Modified classesmatching() function to search parent bundles with inherit => true #5297

Merged
merged 2 commits into from
Jul 27, 2023
Merged

ENT-5850: Modified classesmatching() function to search parent bundles with inherit => true #5297

merged 2 commits into from
Jul 27, 2023

Conversation

craigcomstock
Copy link
Contributor

No description provided.

@craigcomstock craigcomstock marked this pull request as draft July 25, 2023 18:23
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 25, 2023
View reviewed changes
libpromises/eval_context.c Fixed Show fixed Hide fixed
libpromises/eval_context.c Fixed Show fixed Hide fixed
libpromises/evalfunction.c Fixed Show fixed Hide fixed
libpromises/evalfunction.c Fixed Show fixed Hide fixed
@craigcomstock craigcomstock force-pushed the ENT-5850/master branch 2 times, most recently from e71b753 to b233937 Compare July 25, 2023 18:57
@craigcomstock
Copy link
Contributor Author

@cf-bottom jenkins with exotics, why not!

@cf-bottom
Copy link

Alright, I triggered a build:

Build Status

(with exotics)

Jenkins: https://ci.cfengine.com/job/pr-pipeline/9636/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-9636/

@craigcomstock craigcomstock marked this pull request as ready for review July 25, 2023 20:03
@craigcomstock
Copy link
Contributor Author

I'll check the two failures: in windows and is_readable.

larsewi
larsewi requested changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

Nice work 🚀 Only some nitpicks

tests/acceptance/02_classes/02_functions/classesmatching_inherit.cf Show resolved Hide resolved
tests/acceptance/02_classes/02_functions/classesmatching_inherit.cf Outdated Show resolved Hide resolved
tests/acceptance/02_classes/02_functions/classesmatching_inherit.cf Outdated Show resolved Hide resolved
tests/acceptance/02_classes/02_functions/classesmatching_inherit.cf Outdated Show resolved Hide resolved
tests/acceptance/02_classes/02_functions/classesmatching_inherit.cf Outdated Show resolved Hide resolved
tests/acceptance/02_classes/02_functions/classesmatching_inherit.cf Outdated Show resolved Hide resolved
libpromises/eval_context.c Outdated Show resolved Hide resolved
libpromises/eval_context.c Outdated Show resolved Hide resolved
nickanderson
nickanderson previously approved these changes Jul 26, 2023
View reviewed changes
Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

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

Tests look ok to me,

larsewi
larsewi previously approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Make sure it passes Jenkins

tests/acceptance/02_classes/02_functions/classesmatching_inherit.cf Outdated Show resolved Hide resolved
@craigcomstock
Copy link
Contributor Author

LGTM 🚀 Make sure it passes Jenkins

It did already once with two failures: windows in github and is_readable in jenkins on some exotic.

I will manually debug on windows tomorrow and then re-submit to github/jenkins.

@craigcomstock
Modified classesmatching() function to search parent bundles with inh…
c278bcf 
…erit => true

Ticket: ENT-5850
Changelog: title
@craigcomstock craigcomstock dismissed stale reviews from larsewi and nickanderson via c278bcf July 27, 2023 19:06
@craigcomstock
Copy link
Contributor Author

the failure on windows is due to the fact that the github action tests against the most recent master enterprise binaries instead of against the code changes in this PR, so can be ignored since it is a test I am introducing which is failing.

isreadable passed for me on a linux box so known issue on hpux: https://northerntech.atlassian.net/browse/ENT-9930

@craigcomstock craigcomstock merged commit 8219cf7 into cfengine:master Jul 27, 2023
11 of 13 checks passed
@craigcomstock craigcomstock deleted the ENT-5850/master branch July 27, 2023 19:58
@craigcomstock
Copy link
Contributor Author

cherry picks to
3.18.x #5298
3.21.x #5300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickanderson nickanderson nickanderson approved these changes

@larsewi larsewi Awaiting requested review from larsewi

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@craigcomstock @cf-bottom @nickanderson @larsewi