Skip to content
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

Update supported versions in the bin/check-dependencies script #1175

Conversation

YevhenZvieriev
Copy link
Contributor

This PR adds the system requirements of the 2.4.7-p1, 2.4.6-p6, 2.4.5-p8, 2.4.4-p9 versions.

Copy link
Owner

@markshust markshust left a comment

Choose a reason for hiding this comment

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

I noticed this changeset is very large, even though only 3 new versions were added.

What are your thoughts on re-ordering the magento_versions list (and the related common_dependencies) so that versions are listed in ascending order? This way, there will be only two lines modified in each future changeset, making it easier to review the updates to this file. Right now, there is a lot of room for human error.

@YevhenZvieriev
Copy link
Contributor Author

I don't think anyone will make modifications to this script except me, and I test the script thoroughly to avoid mistakes, so it's just my headache xD

But your idea makes sense to me, I'll implement this as soon as possible.

Also, I want to highlight when will is released Magento v2.4.6-p7, I will add these dependencies to this script, 2.4.6-p7 will be approximately within the middle of the list, and adding this also will lead to a changeset that will be very large again.

@YevhenZvieriev
Copy link
Contributor Author

In the last commit, I reordered the magento_version list and related common_dependencies in ascending order.

This should make it easier to review updates and minimize human error.

@markshust
Copy link
Owner

@YevhenZvieriev thanks for the updates! This isn't only for you, but also for me as a reviewer :) Whenever there is a change, without the new ordering, I have no clue if we are introducing regressive changes or etc. with a major diff change. This really helps things!

@markshust markshust merged commit ad8ea0f into markshust:release/next Jul 31, 2024
1 check passed
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