-
Notifications
You must be signed in to change notification settings - Fork 2
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
feature/enhance-pattern-matching #11
base: 1.x-1.x
Are you sure you want to change the base?
feature/enhance-pattern-matching #11
Conversation
Hi @yorkshire-pudding, hope you're well. Just wanted to ask how would one structure a backdrop module that would contain external libraries managed by composer? The way we did this to our other modules is that we created a github actions workflow to build the composer dependencies and package the module including the vendor folder. |
Hi @alvinjohnsonso - There is some guidance on using libraries here - https://docs.backdropcms.org/documentation/using-libraries In general, the approach is to have everything bundles inside the module folder. You would have a subfolder ( Does it need to be managed by composer? Making a dependency for a module to use composer is not encouraged as it raises barriers to collaboration. While lots of people have github actions running tests on PRs, I don't think we use composer. Taking a step back, what is the need for this additional library? I've not have anything that I haven't been able to do with the existing pattern matching. |
Hi @yorkshire-pudding I see. I'll probably just follow that guide you sent. Thanks for answering my question! 😁 As for why we're making this update, we've recently developed a library that provides more flexibility on doing url/path pattern matching to filter which pages to show/hide the widget from. We thought we could also use it on most of the plugins we're integrated to. |
I made the |
@@ -0,0 +1,7 @@ | |||
<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To anyone that might ask why these composer files are included is because Backdrop has its own release workflow that retrieves and builds the module when a release is published. I looked for other modules that has external libraries being used and they all included the library within their repository. Here are some of them:
@alvinjohnsonso - I'm sorry but I can't see that this feature addition is required to the extent that we should increase the complexity in this way. The current pattern matching works well and I have never had any need for anything more complex to meet genuine use cases. I'm not sure what you mean by "Backdrop has its own release workflow that retrieves and builds the module when a release is published". When Backdrop builds a module, it packages the zip, and tar.gz files and updates the .info file in the package zip file. I downloaded the file from your repo branch and was able to install using manual install from that. I think there are too many examples in the tooltip. The config only requires paths from the root leading slash, so why have all the domains? I also don't think that a tooltip is good practice for guidance. I suggest to include in a collapsible section if necessary. I'm afraid I cannot do a code review justice on such a massive increase in code. |
Hi @yorkshire-pudding I understand. This was decided internally and I've asked my superiors to review it. Thanks for the inputs! 😁 |
Added tawk.to pattern matching library and applied it to the module to have a more flexible URL/path rule filtering.