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

[REF] Move ACLPermission Trait into Civi Folder so that it can be acc… #20208

Merged
merged 1 commit into from
May 12, 2021

Conversation

seamuslee001
Copy link
Contributor

…essed by Extensions

Overview

This moves the PermissionTrait from in CRMTraits which doesn't get autoloaded by the Extension test autoloading into the Civi/Test namespace

Before

Trait not autoloaded as CRMTraits folder doesn't get autoloaded when booting tests for extensions

After

Trait in the Civi/Test namespace and gets autoloaded when booting tests for extensions

pint @eileenmcnaughton @colemanw

@civibot
Copy link

civibot bot commented May 3, 2021

(Standard links)

@civibot civibot bot added the master label May 3, 2021
@eileenmcnaughton
Copy link
Contributor

So if this still works for tests then it's mostly OK. The only thing is there is a risk external extensions will start using it & expect it not to change. If we feel it's 'mature' that might be fine - otherwise we could comment some warnings in

@seamuslee001 seamuslee001 force-pushed the move_acl_test_trait branch 2 times, most recently from 8155171 to 9b421c4 Compare May 3, 2021 08:35
@seamuslee001 seamuslee001 force-pushed the move_acl_test_trait branch from 9b421c4 to 57a4d21 Compare May 3, 2021 11:25
@eileenmcnaughton
Copy link
Contributor

OK we discussed this & the class should be fairly stable & anyone using it in tests should have CI to pick up any changes

@eileenmcnaughton eileenmcnaughton merged commit 7a33351 into civicrm:master May 12, 2021
@eileenmcnaughton eileenmcnaughton deleted the move_acl_test_trait branch May 12, 2021 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants