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

improve bump script #3368

Open
wants to merge 2 commits into
base: 5.2-dev
Choose a base branch
from

Conversation

tecpromotion
Copy link
Member

Zusammenfassung der Änderungen

First there was just the idea of passing the date with the -d option. And then after a few iterations, this came out.
Please test this locally and give me feedback.

@tecpromotion tecpromotion requested review from a team, zero-24 and heelc29 and removed request for a team January 11, 2025 21:39
@tecpromotion tecpromotion self-assigned this Jan 11, 2025
build/bump.php Outdated Show resolved Hide resolved

if ($updated) {
file_put_contents($filePath, $contents);
echo "Copyright updated: $filePath" . PHP_EOL;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure whether its worth writing out any change? Maybe its worth adding an "-v" option to show such logging but ignore it in most cases?

$updated = true;
}

if (preg_match('#2008\s+-\s+[0-9]{4}\s+J\!German#', $contents)) {
Copy link
Member

Choose a reason for hiding this comment

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

Please help me but to me it looks like we will always go into this right? Is is possible somehow to check whether that year is != the current year first?

$relativePath = str_replace($rootPath, '', $filePath);

// Skip excluded files and directories
if (preg_match('#\.(png|jpeg|jpg|gif|bmp|ico|webp|svg|woff|woff2|ttf|eot)$#', $filePath) || in_array($relativePath, $excludeFiles)) {
Copy link
Member

Choose a reason for hiding this comment

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

I do get the idea here but wouldn't it be easier to list only the extensions to include rather than the extensions to exclude?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was taken from the original, but of course it should be adapted!

@zero-24
Copy link
Member

zero-24 commented Jan 12, 2025

Left comments looks good on a local test beyond that, but I think we should check that copyright and exclude/include extension thing.

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

Successfully merging this pull request may close these issues.

2 participants