-
-
Notifications
You must be signed in to change notification settings - Fork 630
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
Added features bundle #3
Conversation
yannickl88
commented
Apr 21, 2017
•
edited
Loading
edited
Q | A |
---|---|
License | MIT |
What about also adding a default config file as an example on how to use the bundle? |
I can't really add much other then commented stuff since the bundle does not have any revolvers. |
Service example filename is wrong, should be something like features (not the Vendor name) 😉 |
@sstok you are right, my bad |
@fabpot seems like https://pr3.contrib.symfony.sh/ is not working anymore: If I try: https://pr1.contrib.symfony.sh/ it "works", pr2 is down as well. |
@iltar the URL used by the server changed. It is now https://symfony.sh/r/contrib/3 As the PR has not been updated after the backend change, the URL reported to github is still the old one |
Ah that works for me 👍 |
# my.feature: | ||
# request: ["beta", "on"] | ||
|
||
services: |
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.
@fabpot should we really have the service definition and the semantic configuration in the same file ? Shouldn't services be defined in a separate file ?
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.
In this case, the goal is to make sure services that depend on this package are removed when removing the package, else you would have to remove them manually.
It seems fine to me :/
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.
The classes it refers to, do not exist, they are merely examples. I do agree with keeping them in here, I'm just not entirely sure if the examples should be in here.
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.
do we really need to have this file? see similar comment from @stof in sibling PR: #6 (comment)
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.
These services should be removed, as they should be added by the dev in the main container.yaml
file.
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.
container.yaml
is something that is part of the app? or is it something I need to add?
Either way, this is just example config (and service definition) since the bundle does not provide any implementations. I added this on request of @fabpot (see first comment) Anyhow, the bundle does not throw any errors when nothing is configured so everything is optional, hence why I commented everything. Let me know if I need to remove the config file, change it or keep it as is. |
I think having commented configuration to help people get started seems like a good idea to me. |
so... can this be merged in the current state or do I need to change anything? |
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.
Pull request does not pass validation.
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.
Pull request does not pass validation.
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.
Pull request passes validation.
Thank you for this PR and sorry for the long wait. |
@Nyholm That is the thing, I cannot add a good default since this bundle only adds infrastructure. The functional code has to be made by the person using this. It's like asking for some default config for the form component, you cannot add those without implementing a form type. So at best I can add some commented service definitions, but that is about it. See the readme for an example. |
I understand. Then I suggest closing this PR since Flex is smart enough to install this bundle without a recipe. |
Im closing this PR because of inactivity. Feel free to open a new PR. |
* Add Glide bundle configuration files Added a manifest.json and a dahromy_glide.yaml to configure the Glide bundle in Symfony. The manifest.json includes the bundle and its environment-specific parameters, while the dahromy_glide.yaml sets up source, cache locations and the glide signature key. * # This is the 1st commit message: Add 'gd' driver to dahromy_glide configuration The gd library driver has been added to the dahromy_glide.yaml configuration file. This change enables image processing capabilities provided by the gd library in the dahromy/glide-symfony project. # This is the commit message #2: Add base_url to glide configuration In the dahromy_glide.yaml configuration file, a new line specifying the base_url for image storage has been added. This amendment allows the glide library to know where to store and retrieve images. # This is the commit message #3: Add newline at the end of glide.yaml file A newline has been added at the end of the dahromy_glide.yaml file. This is in accordance with standard coding practices and helps to prevent any potential parsing errors due to the absence of a newline. # This is the commit message #4: Remove 'gd' driver and base_url from glide config The 'gd' driver and the 'base_url' configuration option have been removed from the Glide configuration. Other default configuration options can be added as needed. This change simplifies the default configuration. # This is the commit message #5: Move manifest and config files to 1.0 directory All manifest and config files have been moved to a newly created 1.0 directory. This change is to improve file organization and version control. The structure change does not bring any functionality differences. # This is the commit message #6: Add PHP version conflict in manifest.json The update adds a "conflict" field to the manifest.json which restricts the usage of the package with PHP versions less than 8.0. This ensures that the application is run on the supported PHP versions, preventing any compatibility issues. # This is the commit message #7: Add new project files and update configuration This commit introduces new .idea project files and updates DahRomy Glide Bundle routes and configuration. The various .idea files correspond to Project Modules, Git Toolbox settings and PHP options. Additionally, the Glide configuration has been altered to change the source directory and the signature key has been removed. Add new project files and update configuration This commit introduces new .idea project files and updates DahRomy Glide Bundle routes and configuration. The various .idea files correspond to Project Modules, Git Toolbox settings and PHP options. Additionally, the Glide configuration has been altered to change the source directory and the signature key has been removed. Add newline at end of YAML files and clean up .idea - Add newline at end of YAML configuration files - Remove .idea directory - Add .idea to .gitignore Add newline at end of YAML configuration files Two YAML configuration files in the glide-symfony project lacked a newline character at the end of the file. These have been added to the `glide.yaml` file in both the routes and packages directories, following the POSIX standards for text files. Remove .idea directory Add .idea to .gitignore Remove .gitignore