Skip to content

Conversation

@krejzyOne
Copy link
Contributor

@krejzyOne krejzyOne commented Jan 15, 2024

Přidává podporu pro nette 3.1 a php 8.3

  • instalace phpstan/phpstan-nette kvůli odstranění erroru při pracování s $this->template->link
  • template nemá v nette 3.1 již metodu add, proto přepsáno nette způsob, vytváření proměnných

Copy link

@VojtechBuba VojtechBuba left a comment

Choose a reason for hiding this comment

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

Zkus prosím zvážit navrhované úpravy. Takto by to opravdu mohlo jít ven bez BC breaku.

if ($template instanceof Template) {
$template->add('link', new AsyncControlLink($linkMessage, $linkAttributes));
}
$template->link = new AsyncControlLink($linkMessage, $linkAttributes);

Choose a reason for hiding this comment

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

Tady jsou dvě mezery za rovnítkem

Suggested change
$template->link = new AsyncControlLink($linkMessage, $linkAttributes);
$template->link = new AsyncControlLink($linkMessage, $linkAttributes);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Díky za postřeh. Upraveno

composer.json Outdated
"require": {
"php": "~7.4 | ~8.0",
"nette/application": "^2.4"
"php": "~7.4 | 8.0 - 8.3",

Choose a reason for hiding this comment

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

Ja bych tady nechal ty původní hodnoty. Tímto říkáme, jakou verzi vyžaduje náš kód v knihovně, neřešil bych podporu ostatních knihoven. Když vyjde php 8.4 tak nejspíš bude náš kód stále kompatibilní. Pokud by závislosti přidaly podporu php 8.4 v minor verzích, tak ani nebudeme muset vydávat novou verzi.

Suggested change
"php": "~7.4 | 8.0 - 8.3",
"php": "~7.4 | ~8.0",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

composer.json Outdated
"php": "~7.4 | ~8.0",
"nette/application": "^2.4"
"php": "~7.4 | 8.0 - 8.3",
"nette/application": "^3.0"

Choose a reason for hiding this comment

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

Tady je otázka zdali ten přístup skrze magický setter, nebyl podporovaný už v 2.4. Pokud ano, tak by tady klidně mohly být obě verze, protože knihovně to je jedno a jde nám pouze o kompatibilitu s vyšší verzí nette/application při jejím použití.

Suggested change
"nette/application": "^3.0"
"nette/application": "^2.4 | ^3.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dobrý nápad. Magický setter podporovaný v nette samozřejmě je.
Musel jsem upravit test, aby procházel v obou verzích. nette/application od 3.0 rozšiřuje metodu TemplateFactory::createTemplate o další parametr. Aby byl jeden test tak je třeba ověřovat správnost přes callback v Mockery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants