-
Notifications
You must be signed in to change notification settings - Fork 3
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
OPENEUROPA-480: Provide a language selection page #5
Changes from 6 commits
ed46723
26d9978
a440d50
7b2efeb
73eb7a8
1148e4f
2522db4
723dac9
bcf857b
483e82a
8dd5578
f079c89
cd7662e
cb5f3df
52c18fb
46bb0fe
da72ab3
4465652
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,10 +6,12 @@ | |
"minimum-stability": "dev", | ||
"prefer-stable": true, | ||
"require": { | ||
"php": "^7.1", | ||
"drupal/administration_language_negotiation": "1.4", | ||
"drupal/core": "~8.6", | ||
"drupal/pathauto": "1.2", | ||
"php": "^7.1" | ||
"drupal/devel": "^1.2", | ||
"drupal/language_selection_page": "2.2", | ||
"drupal/pathauto": "1.2" | ||
}, | ||
"require-dev": { | ||
"cweagans/composer-patches": "~1.0", | ||
|
@@ -18,10 +20,11 @@ | |
"drupal/console": "^1.6", | ||
"drupal/config_devel": "~1.2", | ||
"drush/drush": "^9", | ||
"drupal/devel": "^1.2", | ||
"drupal/drupal-extension": "^4.0.0@alpha", | ||
"openeuropa/code-review": "^0.2", | ||
"openeuropa/task-runner": "^0.5", | ||
"openeuropa/oe_theme": "dev-master", | ||
"openeuropa/oe_theme": "dev-OPENEUROPA-480", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before this PR is merged we should get this sister branch in OE Theme merged so this can be reverted back to the master branch. I would also propose to not rely on the master branch but instead making regular releases for the OE Theme. It is clear here that we are relying on a new feature that is being developed in OE Theme: the template for the language switcher. Since we depend on that feature this should be clearly indicated in our dependency list. This information is missing if we say that we depend on
This way we can ensure that the actual template we depend on will be available, and Composer will refuse to build otherwise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would always commit the theme and remove the branch as part of the final commit process. I agree that now the platform is evolving and growing we should start using minimum version increases for components. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, agreed on not using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be taken care of before this is merged into the master branch, or this WILL break the master when the sister PR in OE Theme is merged and the corresponding branch deleted. If we don't address this then the The best way to handle tickets that require dependent changes in multiple components is to make subtasks for each component, and handle them separately. This allows to push the dependent changes through first, then tag a new release and use this in the depending component. |
||
"webflo/drupal-core-require-dev": "8.6.x" | ||
}, | ||
"scripts": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
name: 'OpenEuropa Multilingual Selection Page' | ||
type: module | ||
description: 'Module for OpenEuropa Multilingual Language Selection Page.' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is better to provide a short description of what the module does, rather than parroting the name of the module. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still needs to be addressed. Here's a suggestion for the description:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually this has been addressed in the latest version of the PR. Approved! 👍 |
||
core: 8.x | ||
package: 'OpenEuropa' | ||
|
||
dependencies: | ||
- drupal:language_selection_page | ||
- oe_multilingual:oe_multilingual | ||
|
||
config_devel: | ||
install: | ||
- core.entity_form_display.node.oe_demo_translatable_page.default | ||
- core.entity_view_display.node.oe_demo_translatable_page.default | ||
- field.storage.node.field_oe_demo_translatable_body | ||
- field.field.node.oe_demo_translatable_page.field_oe_demo_translatable_body | ||
- language.content_settings.node.oe_demo_translatable_page | ||
- node.type.oe_demo_translatable_page | ||
- pathauto.pattern.oe_demo_translatable_page | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This defines 7 config files but they are not present in the PR? Were these forgotten? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a copy/paste gone wrong since they are exported in the demo module and should be kept there indeed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh that explains it, thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been fixed, thanks 👍 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
<?php | ||
|
||
/** | ||
* @file | ||
* Install, update and uninstall functions for the module. | ||
*/ | ||
|
||
declare(strict_types = 1); | ||
|
||
use Drupal\administration_language_negotiation\Plugin\LanguageNegotiation\LanguageNegotiationAdministrationLanguage; | ||
use Drupal\language\Plugin\LanguageNegotiation\LanguageNegotiationUrl; | ||
use Drupal\language\Plugin\LanguageNegotiation\LanguageNegotiationSelected; | ||
use Drupal\language_selection_page\Plugin\LanguageNegotiation\LanguageNegotiationLanguageSelectionPage; | ||
|
||
/** | ||
* Implements hook_install(). | ||
*/ | ||
function oe_multilingual_selection_page_install() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing return type declaration ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has not yet been addressed, but it is such a minor issue that I won't block acceptance on this. |
||
// Configure selection page language negotiation method. | ||
\Drupal::configFactory() | ||
->getEditable('language_selection_page.negotiation') | ||
->set('path', '/select-language') | ||
->set('type', 'standalone') | ||
->set('ignore_neutral', FALSE) | ||
->set('blacklisted_paths', [ | ||
'/admin', | ||
'/user', | ||
'/admin/*', | ||
'/admin*', | ||
'/node/add/*', | ||
'/node/*/edit', | ||
])->save(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of setting configuration in an install hook, the recommended way to provide default configuration is by exporting it into a YAML file and placing it in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This configuration is not provided by the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. It would have the same effect, the configuration is in the config/install of the module this module depends on, so that will be installed first, causing the same error. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK let's go with the install hook, but there is a risk involved with this. If the site builder enables this module, then decides not to use it and uninstalls it again, the change to the language negotiation will not be undone. I would not suggest to revert to the previous language negotiation strategy either, since there might be a very long time between the enabling and uninstallation of the module, and the site may have become dependent on the language negotiation provided by this module. What I think we need is a disclaimer / opt-in checkbox when enabling the module, and also documentation in the README that explains to site builders that enabling this module will set the administrative language to English. Is there anything we can revert safely on I don't think we can touch the other things on uninstall though :-/ Providing a disclaimer and opt-in checkbox on installation is out of scope for this ticket, let's do this in a followup after discussion. We can put a |
||
|
||
/** @var \Drupal\oe_multilingual\LanguageNegotiationSetter $setter */ | ||
$setter = \Drupal::service('oe_multilingual.language_negotiation_setter'); | ||
|
||
// For interface negotiation make sure administrative pages are in English. | ||
$setter->setInterfaceSettings([ | ||
LanguageNegotiationAdministrationLanguage::METHOD_ID => -20, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be removed. We should not mess with the site's language negotiation settings simply because the site builder is enabling this small submodule. This could unexpectedly break the language negotiation strategy which is usually very carefully defined by the stakeholders, without a clear indication of what caused this sudden change, and without a way to revert the damage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As explained in the previous comment, let's keep it for now but try to handle whatever we can revert safely in a |
||
LanguageNegotiationUrl::METHOD_ID => -19, | ||
LanguageNegotiationLanguageSelectionPage::METHOD_ID => -18, | ||
LanguageNegotiationSelected::METHOD_ID => 20, | ||
]); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?php | ||
|
||
/** | ||
* @file | ||
* Contains hooks to manipulate language links for selection page. | ||
*/ | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's declare strict typing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not yet done. |
||
/** | ||
* Implements hook_page_preprocess(). | ||
*/ | ||
function oe_multilingual_selection_page_preprocess_language_selection_page_content(array &$variables) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return type declaration is missing ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not yet done but is a minor issue which doesn't block acceptance. |
||
|
||
$current_language = \Drupal::languageManager() | ||
->getCurrentLanguage() | ||
->getId(); | ||
|
||
foreach ($variables['language_links']['#items'] as $key => $value) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this code doing? There is no documentation here to indicate why this is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been addressed, thanks! 👍 |
||
$language_code = $key; | ||
$url = $value['#url']->toString(); | ||
|
||
$variables['languages'][] = [ | ||
"href" => $url, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use single quotes here. |
||
"hreflang" => $language_code, | ||
"label" => $value["#title"], | ||
"lang" => $language_code, | ||
"isActive" => $language_code === $current_language, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With ECL 1.0.0 this should be |
||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,29 +36,31 @@ function oe_multilingual_install() { | |
'/node/*/translations', | ||
])->save(); | ||
|
||
// Make sure that English language prefix is set to "en". | ||
\Drupal::configFactory() | ||
->getEditable('language.negotiation') | ||
->set('url.prefixes.en', 'en') | ||
->save(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems strange, we set the language negotiation for English, but leave the rest untouched? It seems clear to me that the goal of OE Multilingual is to provide the full language negotiation tactic. Just overriding English won't be enough since there are no guarantees that the other settings are matching the defaults from Drupal core. In other words, this will work fine only if the site builder hasn't been messing with the language negotiation before enabling this module. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The other language settings are not coming from core but from our module's About the site builder messing with configuration: we won't have this case since the OpenEuropa components are meant to be enabled at the beginning of the development process, the site configuration will be then exported in the site's I do agree, however, that this might generate confusion, we should either document this or have a talk with the team about this, there is also a ticket about configuration management, we could deal with this in that ticket's scope. |
||
|
||
/** @var \Drupal\oe_multilingual\LanguageNegotiationSetter $setter */ | ||
$setter = \Drupal::service('oe_multilingual.language_negotiation_setter'); | ||
|
||
// Set default language negotiation methods. | ||
$setter->enableNegotiationMethods([ | ||
LanguageInterface::TYPE_INTERFACE, | ||
LanguageInterface::TYPE_CONTENT, | ||
]); | ||
|
||
// For interface negotiation make sure administrative pages are in English. | ||
$interface_settings = [ | ||
$setter->setInterfaceSettings([ | ||
LanguageNegotiationAdministrationLanguage::METHOD_ID => -20, | ||
LanguageNegotiationUrl::METHOD_ID => -19, | ||
LanguageNegotiationSelected::METHOD_ID => 20, | ||
]; | ||
]); | ||
|
||
// For content negotiation make sure that content respects URL language. | ||
$content_settings = [ | ||
$setter->setContentSettings([ | ||
LanguageNegotiationUrl::METHOD_ID => -19, | ||
LanguageNegotiationSelected::METHOD_ID => 20, | ||
]; | ||
|
||
// Set default language negotiation methods with related weights. | ||
\Drupal::configFactory() | ||
->getEditable('language.types') | ||
->set('configurable', [ | ||
LanguageInterface::TYPE_INTERFACE, | ||
LanguageInterface::TYPE_CONTENT, | ||
]) | ||
->set('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.enabled', $interface_settings) | ||
->set('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.method_weights', $interface_settings) | ||
->set('negotiation.' . LanguageInterface::TYPE_CONTENT . '.enabled', $content_settings) | ||
->set('negotiation.' . LanguageInterface::TYPE_CONTENT . '.method_weights', $content_settings) | ||
->save(); | ||
]); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
services: | ||
oe_multilingual.language_negotiation_setter: | ||
class: Drupal\oe_multilingual\LanguageNegotiationSetter | ||
arguments: ['@config.factory'] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
<?php | ||
|
||
namespace Drupal\oe_multilingual; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets declare strict typing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been addressed, thanks! 👍 |
||
|
||
use Drupal\Core\Config\ConfigFactoryInterface; | ||
use Drupal\Core\Language\LanguageInterface; | ||
|
||
/** | ||
* Helper service for language negotiation method configuration. | ||
*/ | ||
class LanguageNegotiationSetter { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's provide an interface for this service so that it can be overridden by custom implementations while still ensuring that the API stays the same. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great, there is an interface now, thanks a lot! 👍 |
||
|
||
/** | ||
* Configuration name. | ||
*/ | ||
const CONFIG_NAME = 'language.types'; | ||
|
||
/** | ||
* Drupal\Core\Config\ConfigFactoryInterface definition. | ||
* | ||
* @var \Drupal\Core\Config\ConfigFactoryInterface | ||
*/ | ||
protected $configFactory; | ||
|
||
/** | ||
* Constructs a new LanguageNegotiationSetter object. | ||
*/ | ||
public function __construct(ConfigFactoryInterface $config_factory) { | ||
$this->configFactory = $config_factory; | ||
} | ||
|
||
/** | ||
* Enable given language negotiation methods. | ||
* | ||
* @param array $methods | ||
* Array of language negotiation method names. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe provide an example here of how the passed data should look? By reading this documentation I have no idea of the kind of method names I should be passing in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK there is ample documentation now, thanks! 👍 |
||
*/ | ||
public function enableNegotiationMethods(array $methods) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return type declaration is missing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been addressed, thanks! 👍 |
||
$this->configFactory | ||
->getEditable(self::CONFIG_NAME) | ||
->set('configurable', $methods) | ||
->save(); | ||
} | ||
|
||
/** | ||
* Set interface language negotiation settings. | ||
* | ||
* @param array $settings | ||
* Array of language negotiation method names with their weights. | ||
*/ | ||
public function setInterfaceSettings(array $settings): void { | ||
$this->configFactory | ||
->getEditable(self::CONFIG_NAME) | ||
->set('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.enabled', $settings) | ||
->set('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.method_weights', $settings) | ||
->save(); | ||
} | ||
|
||
/** | ||
* Set content language negotiation settings. | ||
* | ||
* @param array $settings | ||
* Array of language negotiation method names with their weights. | ||
*/ | ||
public function setContentSettings(array $settings): void { | ||
$this->configFactory | ||
->getEditable(self::CONFIG_NAME) | ||
->set('negotiation.' . LanguageInterface::TYPE_CONTENT . '.enabled', $settings) | ||
->set('negotiation.' . LanguageInterface::TYPE_CONTENT . '.method_weights', $settings) | ||
->save(); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
|
||
namespace Drupal\Tests\oe_multilingual\Behat; | ||
|
||
use Behat\Behat\Hook\Scope\AfterScenarioScope; | ||
use Behat\Behat\Hook\Scope\BeforeScenarioScope; | ||
use Behat\Gherkin\Node\TableNode; | ||
use Drupal\Core\Entity\ContentEntityInterface; | ||
use Drupal\DrupalExtension\Context\RawDrupalContext; | ||
|
@@ -15,6 +17,33 @@ | |
*/ | ||
class DrupalContext extends RawDrupalContext { | ||
|
||
/** | ||
* Enable OpenEuropa Multilingual Selection Page module. | ||
* | ||
* @param \Behat\Behat\Hook\Scope\BeforeScenarioScope $scope | ||
* The Hook scope. | ||
* | ||
* @BeforeScenario @selection-page | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very nice idea! |
||
*/ | ||
public function setupSelectionPage(BeforeScenarioScope $scope): void { | ||
\Drupal::service('module_installer')->install(['oe_multilingual_selection_page']); | ||
} | ||
|
||
/** | ||
* Disable OpenEuropa Multilingual Selection Page module. | ||
* | ||
* @param \Behat\Behat\Hook\Scope\AfterScenarioScope $scope | ||
* The Hook scope. | ||
* | ||
* @AfterScenario @selection-page | ||
*/ | ||
public function revertSelectionPage(AfterScenarioScope $scope): void { | ||
\Drupal::service('module_installer')->uninstall([ | ||
'oe_multilingual_selection_page', | ||
'language_selection_page', | ||
]); | ||
} | ||
|
||
/** | ||
* Create content given its type and fields. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?php | ||
|
||
namespace Drupal\Tests\oe_multilingual\Behat; | ||
|
||
use Behat\MinkExtension\Context\RawMinkContext; | ||
|
||
/** | ||
* Class MinkContext. | ||
*/ | ||
class MinkContext extends RawMinkContext { | ||
|
||
/** | ||
* Assert that visitor is redirected to language selection page. | ||
* | ||
* @Then I should be redirected to the language selection page | ||
*/ | ||
public function assertLanguageSelectionPageRedirect() { | ||
$this->assertSession()->addressMatches("/.*\/select-language/"); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
@api @selection-page | ||
Feature: Language selection | ||
In order to be able choose the initial language of the site | ||
As a visitor | ||
I want to be presented with a language selection page | ||
|
||
Background: | ||
Given the following "Demo translatable page" content item: | ||
| Title | Test page | | ||
| Body | Hello world | | ||
And the following "French" translation for the "Demo translatable page" with title "Test page": | ||
| Title | Page de test | | ||
| Body | Bonjour le monde | | ||
And the following "Spanish" translation for the "Demo translatable page" with title "Test page": | ||
| Title | Página de prueba | | ||
| Body | Hola Mundo | | ||
|
||
Scenario: When I visit the homepage I'm presented with a language selection page | ||
|
||
Given I am on the homepage | ||
Then I should be redirected to the language selection page | ||
|
||
When I click "French" | ||
Then the url should match "/fr" | ||
|
||
Scenario: Users visiting a page should be presented with the language selection page, | ||
if no language is detected in the URL. | ||
|
||
Given I visit the "Test page" content | ||
Then I should be redirected to the language selection page | ||
|
||
When I click "French" | ||
Then the url should match "/fr/page-de-test" |
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 are no good reasons to lock the versions of the Language Selection Page and Pathauto modules, unless there is a known problem with a newer version. In that case this should be documented here (e.g. by using a "_readme" entry), and followup issues should be made to fix the bug and unlock the version again.
Drupal modules are not yet following semantic versioning (but this is in the works, ref. https://www.drupal.org/project/drupal/issues/1612910), but they do follow the core principles of semantic versioning: new minor versions of modules should only contain bug fixes and new features which do not break backwards compatibility. This means that it can be considered safe to unlock the versions, and it is recommended because it allows us to benefit from bugfixes and security fixes without making any changes to the 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.
Do we have a policy or rule on when to use ^2.2 and when to use ~2.2 ? The current file has a mixture of both.
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.
@pfrenssen agreed, this will also allow site developers to fix the version they want to use in the site's
composer.json
, in case they need it, while if we lock them on the components'composer.json
that would not be possible.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.
Regarding the
~2.2
and^2.2
both are valid so they are both OK to be used. I don't think there is a need to define a policy because there is no "better" way to do this. Both cases are really easy to understand: the tilde updates only the most minor version specified, while the caret updates all versions except the most major one.I personally prefer the tilde for dependencies that use a
<major version>.<minor version>
naming scheme such as Drupal modules. The caret is definitely the one to use for packages that adhere to semantic versioning<major version>.<minor version>.<patch>
.Some people like to use the caret for everything because it is "less confusing for newbies", but I think it is condescending to insinuate that young developers are not able to understand the difference between the two.
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.
This has been addressed, thanks 👍
I see that the
drupal/administration_language_negotiation
dependency is also locked but this is out of scope for this ticket, I created a followup to do this: OPENEUROPA-601.