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

Updated license info in copyright docblocks #3120

Merged
merged 4 commits into from
Apr 4, 2023
Merged

Updated license info in copyright docblocks #3120

merged 4 commits into from
Apr 4, 2023

Conversation

fballiano
Copy link
Contributor

@fballiano fballiano commented Mar 28, 2023

All files start with this code:

/**
 * OpenMage
 * 
 * NOTICE OF LICENSE
 *
 * This source file is subject to the Open Software License (OSL 3.0)
 * that is bundled with this package in the file LICENSE.txt.
 * It is also available through the world-wide-web at this URL:
 * https://opensource.org/licenses/osl-3.0.php
 * If you did not receive a copy of the license and are unable to
 * obtain it through the world-wide-web, please send an email
 * to license@magento.com so we can send you a copy immediately.

but I think that * NOTICE OF LICENSE is redundant and could be removed, sparing a lot of bytes.

I checked a few famous php projects on github and never found anything similar.

EDIT: after a comment from @kiatng I thought we could reduce that block even more (and spare some disk space) to

 * OpenMage
 *
 * This source file is subject to the Open Software License (OSL 3.0)
 * that is bundled with this package in the file LICENSE.txt.
 * It is also available at https://opensource.org/license/osl-3-0-php

@github-actions github-actions bot added Component: AdminNotification Relates to Mage_AdminNotification Component: Adminhtml Relates to Mage_Adminhtml Component: Admin Relates to Mage_Admin Component: Api PageRelates to Mage_Api Component: Api2 Relates to Mage_Api2 Component: Authorizenet Relates to Mage_Authorizenet Component: Bundle Relates to Mage_Bundle Component: Captcha Relates to Mage_Captcha Component: Catalog Relates to Mage_Catalog Component: CatalogInventory Relates to Mage_CatalogInventory Component: CatalogRule Relates to Mage_CatalogRule Component: CatalogIndex Relates to Mage_CatalogIndex Component: CatalogSearch Relates to Mage_CatalogSearch Component: Centinel Relates to Mage_Centinel Component: Checkout Relates to Mage_Checkout Component: Cms Relates to Mage_Cms Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches Component: Contacts Relates to Mage_Contacts Component: Core Relates to Mage_Core Component: Cron Relates to Mage_Cron Component: CurrencySymbol Relates to Mage_CurrencySymbol Component: Customer Relates to Mage_Customer Component: Dataflow Relates to Mage_Dataflow Component: Directory Relates to Mage_Directory Component: Downloadable Relates to Mage_Downloadable Mage.php Relates to app/Mage.php labels Mar 28, 2023
@fballiano
Copy link
Contributor Author

the workflows break because too many files were changed, but if you check the diff https://github.com/OpenMage/magento-lts/pull/3120.diff you'll see there's nothing apart from the comment change

Copy link

@splendidinternet splendidinternet left a comment

Choose a reason for hiding this comment

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

ja, sure, claro.

@fballiano
Copy link
Contributor Author

no comments? if @admins don't like this i'll close it, I really don't like when PRs get zero feedback from anyone.

@fballiano
Copy link
Contributor Author

can I get some feedback by @elidrissidev @kiatng @addison74? thanks guys

@kiatng
Copy link
Contributor

kiatng commented Apr 3, 2023

In addition, how about reduce it further from

 * It is also available through the world-wide-web at this URL:
 * https://opensource.org/licenses/osl-3.0.php
 * If you did not receive a copy of the license and are unable to
 * ...

to just

 * It is also available through the world-wide-web at this URL:
 * https://github.com/OpenMage/magento-lts/

and remove the following lines about other ways to obtain the license.

@fballiano
Copy link
Contributor Author

In addition, how about reduce it further from

 * It is also available through the world-wide-web at this URL:
 * https://opensource.org/licenses/osl-3.0.php
 * If you did not receive a copy of the license and are unable to
 * ...

to just

 * It is also available through the world-wide-web at this URL:
 * https://github.com/OpenMage/magento-lts/

I'd leave the opensource.org link just for independentness (dunno if it's even a word) but I'd write It is also available at https://opensource.org/licenses/osl-3.0.php instead of through the world-wide-web (which seem a very outdated form of speech)

and remove the following lines about other ways to obtain the license.

I didn't want to change the text that much (licensing is always tricky) but... what the heck, it's about time we remove it, and I don't think it's against the license anyway.

@fballiano
Copy link
Contributor Author

I've updated and reduced the block even more, also updating the links to the licenses (there were changed on the opensource.org website).

@fballiano fballiano changed the title Removed "notice of license" line in docblock since it is redundant Updated license docblocks Apr 3, 2023
Copy link
Contributor

@justinbeaty justinbeaty left a comment

Choose a reason for hiding this comment

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

I am good with the change, let's just make sure to have the commit message match the following regex. That is so the update-copyright.php script will not update all files due to this change only. So, "Updated copyright docblocks" would work.

/^(Add(ed)?|Update(d)?).*?Copyright/i

I verified by:

  1. Check to see only comment lines were changed:
 git diff -U0 HEAD~2 | grep '^[+-]' | grep -Ev '^(--- a/|\+\+\+ b/)' | sort -u
- * https://opensource.org/licenses/afl-3.0.php
-// https://opensource.org/licenses/afl-3.0.php
- * https://opensource.org/licenses/osl-3.0.php
-# https://opensource.org/licenses/osl-3.0.php
- * If you did not receive a copy of the license and are unable to
-# If you did not receive a copy of the license and are unable to
-// If you did not receive a copy of the license and are unable to
+ * It is also available at https://opensource.org/license/afl-3-0-php
+// It is also available at https://opensource.org/license/afl-3-0-php
+ * It is also available at https://opensource.org/license/osl-3-0-php
+# It is also available at https://opensource.org/license/osl-3-0-php
- * It is also available through the world-wide-web at this URL:
-# It is also available through the world-wide-web at this URL:
-// It is also available through the world-wide-web at this URL:
- * obtain it through the world-wide-web, please send an email
-# obtain it through the world-wide-web, please send an email
-// obtain it through the world-wide-web, please send an email
- * to license@magento.com so we can send you a copy immediately.
-# to license@magento.com so we can send you a copy immediately.
-// to license@magento.com so we can send you a copy immediately.
  1. Check to see we didn't miss anything:
git grep 'It is also available through the world-wide-web'
git grep 'license@magento.com'

@fballiano
Copy link
Contributor Author

@justinbeaty can that be done when squashing and merging? I think so.

@justinbeaty
Copy link
Contributor

@fballiano Yes for sure. Just wanted to make sure to mention it. Worst case scenario if the commit message didn't match one of the regexes, we just modify the update-copyright.php script to ignore that commit by hash or updated regex.

@fballiano
Copy link
Contributor Author

@justinbeaty you're absolutely right cause I wouldn't have thought of that, could you directly send me a commit comment that I could use in case we get this approved?

@justinbeaty
Copy link
Contributor

@fballiano something like one of these:

Update copyright docblocks
updated license info in copyright docblocks
update start of file to simplify copyright

Basically, start the commit message with the word update/updated and also use the word copyright in the message. Example: https://regex101.com/r/xye3Qw/1

@fballiano
Copy link
Contributor Author

perfect, thank you!

@fballiano fballiano merged commit d1ef6e4 into OpenMage:1.9.4.x Apr 4, 2023
@fballiano fballiano changed the title Updated license docblocks Updated license info in copyright docblocks Apr 4, 2023
@fballiano fballiano deleted the noticeoflicense branch April 4, 2023 07:27
@fballiano
Copy link
Contributor Author

merged and v20ed, thanks everybody!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Admin Relates to Mage_Admin Component: Adminhtml Relates to Mage_Adminhtml Component: AdminNotification Relates to Mage_AdminNotification Component: Api PageRelates to Mage_Api Component: Api2 Relates to Mage_Api2 Component: Authorizenet Relates to Mage_Authorizenet Component: Bundle Relates to Mage_Bundle Component: Captcha Relates to Mage_Captcha Component: Catalog Relates to Mage_Catalog Component: CatalogIndex Relates to Mage_CatalogIndex Component: CatalogInventory Relates to Mage_CatalogInventory Component: CatalogRule Relates to Mage_CatalogRule Component: CatalogSearch Relates to Mage_CatalogSearch Component: Centinel Relates to Mage_Centinel Component: Checkout Relates to Mage_Checkout Component: Cms Relates to Mage_Cms Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches Component: Contacts Relates to Mage_Contacts Component: Core Relates to Mage_Core Component: Cron Relates to Mage_Cron Component: CurrencySymbol Relates to Mage_CurrencySymbol Component: Customer Relates to Mage_Customer Component: Dataflow Relates to Mage_Dataflow Component: Directory Relates to Mage_Directory Component: Downloadable Relates to Mage_Downloadable Mage.php Relates to app/Mage.php
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants