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

Cleanup file headers #3358

Closed
khassel opened this issue Jan 16, 2024 · 7 comments
Closed

Cleanup file headers #3358

khassel opened this issue Jan 16, 2024 · 7 comments

Comments

@khassel
Copy link
Collaborator

khassel commented Jan 16, 2024

See discussion here.

Beside the author headers: Need we in every file the MIT Licensed stuff?

Any other cleanup wishes?

@khassel
Copy link
Collaborator Author

khassel commented Jan 16, 2024

For the author headers: find ./ -type f -exec sed -i -r '/^\s\* By.*$/d' {} \;

@KristjanESPERANTO
Copy link
Contributor

KristjanESPERANTO commented Jan 16, 2024

In my understanding of MIT, it is not necessary to mention the license in all files. But I think it's fine to do it.

As a basis for discussion, here is the current header from app.js:

   /* MagicMirror²
   * The Core App (Server)
   *
   * By Michael Teeuw https://michaelteeuw.nl
   * MIT Licensed.
   */

And here three spontaneous proposals:

Proposal 1:
author line removed + asterisk below each other

  /*
   * MagicMirror²
   * The Core App (Server)
   *
   * MIT Licensed.
   */

Proposal 2:
author line removed + license note formulated in more detail + asterisk below each other

 /*
  * MagicMirror²
  * The Core App (Server)
  *
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  */

Proposal 3:
author line and license note removed + asterisk below each other

 /*
  * MagicMirror²
  * The Core App (Server)
  */

Feel free to tear these proposals apart. The license part in proposal 2 I found in the sources of react.

@KristjanESPERANTO
Copy link
Contributor

KristjanESPERANTO commented Jan 16, 2024

There are even a few ESLint plugins that check the headers. Here is the rule of one of them that seems suitable: https://github.com/nikku/eslint-plugin-license-header/blob/master/docs/rules/header.md

The example there looks very similar to proposal 2.

  /**
   * Copyright (c) Foo Corp.
   *
   * This source code is licensed under the WTFPL license found in the
   * LICENSE file in the root of this projects source tree.
   */

Edit: But with the ESLint plugin approach, I now see the downside that the headers have to be the same everywhere... Additions like "The Core App (Server)" couldn't be in that block.

@rejas
Copy link
Collaborator

rejas commented Jan 18, 2024

Proposal 4: Remove headers alltogether. They provide no real information. I would rather have proper jsdoc headers everywhere ;-)

@rejas rejas changed the title cleanup file headers Cleanup file headers Jan 18, 2024
@khassel
Copy link
Collaborator Author

khassel commented Jan 18, 2024

from my side we can remove all these headers ...

@KristjanESPERANTO
Copy link
Contributor

I'm also fine with removing 👍

rejas pushed a commit that referenced this issue Jan 24, 2024
see #3358

used command: `find ./ -type f -exec perl -i -0pe
's/\/\*\s*magicmirror.*?\*\/\s*//si' {} \;`

This is a first draft, I think we should preserve some of the comments.
@rejas
Copy link
Collaborator

rejas commented Mar 21, 2024

so I guess this is done and can be closed @khassel ?

@khassel khassel closed this as completed Mar 21, 2024
@rejas rejas mentioned this issue Apr 1, 2024
rejas added a commit that referenced this issue Apr 1, 2024
## [2.27.0] - 2024-04-01

Thanks to: @bugsounet, @crazyscot, @illimarkangur, @jkriegshauser,
@khassel, @KristjanESPERANTO, @Paranoid93, @rejas, @sdetweil and
@vppencilsharpener.

This release marks the first release without Michael Teeuw (@MichMich).
A very special thanks to him for creating MagicMirror and leading the
project for so many years.

For more info, please read the following post: [A New Chapter for
MagicMirror: The Community Takes the
Lead](https://forum.magicmirror.builders/topic/18329/a-new-chapter-for-magicmirror-the-community-takes-the-lead).

### Added

- Output of system information to the console for troubleshooting (#3328
and #3337), ignore errors under aarch64 (#3349)
- [chore] Add `eslint-plugin-package-json` to lint the `package.json`
files (#3368)
- [weather] `showHumidity` config is now a string describing where to
show this element. Supported values: "wind", "temp", "feelslike",
"below", "none". (#3330)
- electron-rebuild test suite for electron and 3rd party modules
compatibility (#3392)
- Create MM² icon and attach it to electron process (#3407)

### Updated

- Update updatenotification (update_helper.js): Recode with pm2 library
(#3332)
- Removing lodash dependency by replacing merge by spread operator
(#3339)
- Use node prefix for build-in modules (#3340)
- Rework logging colors (#3350)
- Update pm2 to v5.3.1 with no allow-ghsas (#3364)
- [chore] Update husky and let lint-staged fix ESLint issues
- [chore] Update dependencies including electron to v29 (#3357) and
node-ical
- Update translations for estonian (#3371)
- Update electron to v29 and update other dependencies
- [calendar] fullDay events over several days now show the left days
from the first day on and 'today' on the last day
- Update layout of current weather indoor values

### Fixed

- Correct apibase of weathergov weatherprovider to match documentation
(#2926)
- Worked around several issues in the RRULE library that were causing
deleted calender events to still show, some
initial and recurring events to not show, and some event times to be off
an hour. (#3291)
- Skip changelog requirement when running tests for dependency updates
(#3320)
- Display precipitation probability when it is 0% instead of blank/empty
(#3345)
- [newsfeed] Suppress unsightly animation cases when there are 0 or 1
active news items (#3336)
- [newsfeed] Always compute the feed item URL using the same helper
function (#3336)
- Ignore all custom css files (#3359)
- [newsfeed] Fix newsfeed stall issue introduced by #3336 (#3361)
- Changed `log.debug` to `log.log` in `app.js` where logLevel is not set
because config is not loaded at this time (#3353)
- [calendar] deny fetch interval < 60000 and set 60000 in this case
(prevent fetch loop failed) (#3382)
- added message in case where config.js is missing the module.export
line PR #3383
- Fixed an issue where recurring events could extend past their
recurrence end date (#3393)
- Don't display any `npm WARN <....>` on install (#3399)
- Fixed move suncalc dependency to production from dev, as it is used by
clock module
- [compliments] Fix mirror not responding anymore when no compliments
are to be shown (#3385)

### Deleted

- Unneeded file headers (#3358)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Michael Teeuw <[email protected]>
Co-authored-by: Kristjan ESPERANTO <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Karsten Hassel <[email protected]>
Co-authored-by: Ross Younger <[email protected]>
Co-authored-by: Bugsounet - Cédric <[email protected]>
Co-authored-by: jkriegshauser <[email protected]>
Co-authored-by: illimarkangur <[email protected]>
Co-authored-by: sam detweiler <[email protected]>
Co-authored-by: vppencilsharpener <[email protected]>
Co-authored-by: Paranoid93 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants