-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
add sylius version to the footer in admin #8680
Conversation
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.
I'm not sure about this new const APP_NAME
. I think we can just hardcode Sylius
because a link will open a Sylius website anyway. A developer will need to override block one way or another.
It's also a quasi bc-break because footer
block is not used anyway.
let's take them one by one:
|
I totally agree with the second point. I just don't think that we need this |
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.
I think that if we can avoid additional constant without any problems - let's avoid it ;) I also don't see a really big benefit of introducing it. Nevertheless, nice work ;)
ea5db83
to
fee8bdc
Compare
OK, I have reverted that change introducing the constant. |
Couldn't spot any BC break here, is it still there? /cc @Zales0123 |
For me it is good to go |
app/config/config.yml
Outdated
@@ -61,3 +61,8 @@ fos_rest: | |||
rules: | |||
- { path: '^/api/.*', priorities: ['json', 'xml'], fallback_format: json, prefer_extension: true } | |||
- { path: '^/', stop: true } | |||
|
|||
twig: |
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.
Can we add it to @CoreBundle/Resources/config/app/config.yml
instead? Btw. we can name it sylius_meta
.
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.
👍
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.
done.
Thanks Gabi! |
add sylius version to the footer in admin
It seems the version was missing from admin due to a bug in a twig file - the footer block in admin got renamed post_content at some point in history.
On top of fixing this bug, I added a new constant in the kernel class, APP_NAME with the default value being "Sylius". In case at a later point someone decides to craft a forked version of Sylius called "Supercharged Sylius" and it needs to brag about the name. Before hand, Sylius was hardcoded in the twig template.
I have declared both Sylius\Bundle\CoreBundle\Application\Kernel::VERSION and Sylius\Bundle\CoreBundle\Application\Kernel::APP_NAME as twig global variables and used them to be in the twig layout template for admin.
I consider this useful when debugging a Sylius application over the phone with a non-technical person. First question I would ask is: what version of Sylius do you run?
Also, I find it a good reason to brag about: look, I'm running Sylius 1.0.0!