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

push: Add push rule to ignore room server ACLs #1242

Merged
merged 2 commits into from
Jul 16, 2022

Conversation

zecakeh
Copy link
Contributor

@zecakeh zecakeh commented Jul 16, 2022

@zecakeh zecakeh requested a review from a team July 16, 2022 10:06
@zecakeh
Copy link
Contributor Author

zecakeh commented Jul 16, 2022

It seems like the indexset! macro doesn't like to have feature flags inside its definition and tries to insert an empty value in the set.

I don't understand how that's possible… Shouldn't the feature flag leave or remove the line below, so it should be transparent for the macro?

@jplatte
Copy link
Member

jplatte commented Jul 16, 2022

Attributes (cfg or otherwise) can't be transparent to function-like macros or attribute macros as there is no way for the compiler to know how much of the macro input the attribute would apply to.

@zecakeh
Copy link
Contributor Author

zecakeh commented Jul 16, 2022

Ah right. How can we solve this then? I don't think that creating a set for every feature flag combination is a good idea…

It seems like in recent versions we can create an IndexSet from an array.

@jplatte
Copy link
Member

jplatte commented Jul 16, 2022

Using an array literal seems good, otherwise we can use mutation to first create the initial set, then add elements conditionally.

Copy link
Member

@ShadowJonathan ShadowJonathan left a comment

Choose a reason for hiding this comment

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

LGTM

Allows more flexibility when several feature flags change the same set.
@zecakeh zecakeh force-pushed the zecakeh/pushrule-serveracl branch from f0e7592 to 3f670ea Compare July 16, 2022 17:02
@zecakeh zecakeh enabled auto-merge (rebase) July 16, 2022 17:03
@zecakeh zecakeh merged commit 2f96fa5 into main Jul 16, 2022
@zecakeh zecakeh deleted the zecakeh/pushrule-serveracl branch July 16, 2022 17:24
@zecakeh zecakeh mentioned this pull request Sep 4, 2022
54 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants