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

Use double slash instead of triple slash in file header template #264

Closed
chykon opened this issue Feb 11, 2023 · 11 comments · Fixed by #402
Closed

Use double slash instead of triple slash in file header template #264

chykon opened this issue Feb 11, 2023 · 11 comments · Fixed by #402
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@chykon
Copy link
Contributor

chykon commented Feb 11, 2023

Motivation

Doc comment (triple slash) for a header can cause the VSCode tooltip to contain unnecessary information.

Снимок экрана от 2023-02-11 13-52-23

Desired solution

Use double slash instead of triple slash in file header template.

Alternatives considered

Ignore it.

Additional details

If there are import statements after the header, then the extra information is not shown.

@chykon chykon added the enhancement New feature or request label Feb 11, 2023
@chykon
Copy link
Contributor Author

chykon commented Feb 11, 2023

And one more thing. Is there a reason why headers have a "tail" (empty ///)?

/// Author: Max Korbel <max.korbel@intel.com>
///

@chykon
Copy link
Contributor Author

chykon commented Feb 11, 2023

Well, and more about the template: is there a reason why you need to indicate the time of creation of the file, authorship and file name if this data is stored in the version control system?

@mkorbel1
Copy link
Contributor

You're right, double slash seems more appropriate for the file headers. I looked at some of the dart-sdk code and they use double slash on their file headers as well.

I think in some files we had put a single // between the header and the definition in order to avoid that extra doc stuff showing up in case there were no import statements.

The tail was just a style preference which I'm not that sure about anymore. Originally, I think I used /* */ style block-comments at the top of files and had no comment content on the first and last lines. Eventually there was some guidance to put copyright and license identifier information right at the top, so the blank head line got removed, but the tails didn't. I'm not attached to the tails and wouldn't really object to removing them.

The creation time and authorship are nice to stick in a file header because even though within the repository there's version control tracking, when files get copied to other locations it leaves a breadcrumb about where they came from. I admit that these become less relevant, accurate, and useful as the files get modified over time, especially in an open-source project. I think I'm not really opposed to them being modified/removed over time as files go through meaningful changes or omitted when new files are created, but I'm also thinking it's perhaps unnecessary for any sort of "sweep" to remove existing ones all at once. I'm open to other thoughts and opinions on the matter.

We should apply changes to https://github.com/intel/rohd-vf and https://github.com/intel/rohd-cosim as well.

@mkorbel1 mkorbel1 added the documentation Improvements or additions to documentation label Feb 13, 2023
@mkorbel1
Copy link
Contributor

I think the fix for this issue should also update guidelines in CONTRIBUTING.md

@chykon
Copy link
Contributor Author

chykon commented Feb 15, 2023

I'll make some other suggestions that come to mind:

  • Usually (in the same DartSDK) they use a small c: (c) Instead of (C). Can use it?
  • Can add a comma: Copyright (c) 2023, Intel Corporation?
  • Can the template be reformulated so that it is in the form of text (as in the DartSDK code) and not a list?
  • Can stop writing the year the file was last modified? Every year it creates a wave of little noise. In the same DartSDK, only the year the file was created is indicated.

@mkorbel1
Copy link
Contributor

Usually (in the same DartSDK) they use a small c: (c) Instead of (C). Can use it?
Can add a comma: Copyright (c) 2023, Intel Corporation?
Can the template be reformulated so that it is in the form of text (as in the DartSDK code) and not a list?

These formats on the copyright statement come from a standard template that Intel provided. I think in this case consistency with Intel guidelines is preferable to consistency with the Dart SDK. Unless there's some additional motivation?

Can stop writing the year the file was last modified?

From a bit of googling, it sounds like it's typical to update the copyright year each time it is modified/published so copyright protection expiration can be easily determined. I'm not sure if it's necessary for us to update it each time, though. I'll ask around if there's any specific Intel guidance on this.

@chykon
Copy link
Contributor Author

chykon commented Feb 15, 2023

These formats on the copyright statement come from a standard template that Intel provided. I think in this case consistency with Intel guidelines is preferable to consistency with the Dart SDK. Unless there's some additional motivation?

Of course, if there are established recommendations, then we use them. Just outlined some ideas, suddenly something can be reconsidered.

@mkorbel1
Copy link
Contributor

Sure, thank you for the thought and suggestions! I appreciate the effort to make the codebase cleaner and more consistent.

@mkorbel1
Copy link
Contributor

I've received guidance to update the year in the copyright as files are updated as a best practice for open-source software. Part of the reasoning is that open-source files can frequently appear outside of their original repos.

@chykon
Copy link
Contributor Author

chykon commented Feb 16, 2023

Thanks for the clarifications!

As a result (for now): what will be the updated template? If I understand correctly, it will be something like this:

// Copyright (C) 2021-2023 Intel Corporation
// SPDX-License-Identifier: BSD-3-Clause
//
// example.dart
// A very basic example of a counter module.

Or will the file name be omitted?

// Copyright (C) 2021-2023 Intel Corporation
// SPDX-License-Identifier: BSD-3-Clause
//
// A very basic example of a counter module.

You can shrink even more by removing the indentation.

// Copyright (C) 2021-2023 Intel Corporation
// SPDX-License-Identifier: BSD-3-Clause
// A very basic example of a counter module.

@mkorbel1
Copy link
Contributor

Personally, I like the file name in there and space between the boilerplate and interesting content (first option in your previous comment)

I think having original author and creation date doesn't hurt, but also is fine to make it not a requirement. Removing/appending author and removing creation date after significant changes to a file I think would make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants