search

pez-globo / pufferfish-software

All software for the Pufferfish ventilator.
Apache License 2.0
0 stars 1 forks source link

Standardize file headers #28

Open ethanjli opened 4 years ago

ethanjli commented 4 years ago

We will need to maintain a consistent style of file headers (including copyright and open-source license notices) to be included across all source files; for example, currently file headers are not consistent across source files in the firmware; and for example, currently the backend and frontend files do not include any copyright or open-source license notices.

The format of the headers should be chosen to be in a reasonable style for each domain (e.g. Python file headers will be structured differently than C++ file headers). As for the copyright and licensing notices which need to go into these headers, refer to https://www.notion.so/pufferfish/Internal-Instructions-for-Use-e8a09fae6f1b46baa22f78fcb6cffc18#d5f45a3d35304ceb9e991669f7d14e11 for the notice which needs to be included in the header. All other header content should be chosen to make sense for each domain, while keeping maintenance overhead low - if it makes sense, the header might only have copyright and licensing notices. For example, we might remove specific author lists or file names from the file headers of the firmware source files. As another example, we might want to include a place in the header to briefly describe the purpose and/or contents of the file (e.g. in Python the standard practice for modules is a one-line description of the module in the docstring at the top of the file).

As a first step, it would be great if everyone proposed a template for the header to add to each file, in their corresponding software domain (firmware vs. backend vs. frontend), and then we can make sure things are reasonably consistent across domains.

This is a low-priority item.

RLaxmanrao commented 4 years ago

@ethanjli Templates are shared over the slack. Kindly have a look and let me know if any changes are required

Sudhir-dev commented 4 years ago 
/****************************************************************************
* @license 
* Copyright (c) 2020 Pez-Globo and the Pufferfish project contributors
* SPDX-License-Identifier: Apache-2.0
* 
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*   http:*www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
*
* See the License for the specific language governing permissions and
* limitations under the License.
*
* @summary short description for the file
****************************************************************************/
ethanjli commented 4 years ago

@Sudhir-dev This generally looks good to me! A few minor details & follow-up questions:

  1. The // in http://www.apache.org got automatically replaced with *, so that needs to be changed back
  2. This looks like a jsdoc-style comment - should we plan to use jsdoc to document the rest of our frontend code too, then? I'll defer to you on what we should use.
  3. In this header template, the license section is in front of the summary section. This will be the same for the C++ header template for the firmware, but it will be different in the Python backend section (since Python wants the short one-line description as the first line of the header docstring); but I think this will be fine. I agree with putting the license first in the javascript file header.
  4. Right now there is no leading space in the lines which start with an asterisk (e.g. * @license rather than * @license). I would suggest adding that leading space, since the examples on jsdoc's homepage use that - for example: 
    /**
* Represents a book.
* @constructor
*/

    and not:

    /**
* Represents a book.
* @constructor
*/

And here's a not-so-minor question: In some files we might use code copy-pasted from another project or otherwise from a third party; this seems much less likely in JS than in C++, but it would still be good to have a strategy if/when it comes up. Here's an example of the file header for a file where we need to include copyright and license notices on the original work: https://github.com/pez-globo/pufferfish-vent-software/blob/develop/firmware/ventilator-controller-stm32/Core/Inc/Pufferfish/Driver/I2C/SDP.h (note that this header is missing the Apache license text for our project). When we have files like that, do you have any ideas on how we might organize our header to include both the original work copyright+license information and our modified work copyright+license information?

Tanmay06 commented 4 years ago

I found this repo which has some standard license templates. We can also refer to OpenPose License for building our own template, as it has examples of both the original work copyright+license information and modified work copyright+license information.

Apart from that, for just python backend we can follow PEP 566 and use python core metadata specifications.

Sudhir-dev commented 4 years ago

Yes, I think we could go with jsdoc. Added leading space on each line

/****************************************************************************
 * @license 
 * Copyright (c) 2020 Pez-Globo and the Pufferfish project contributors
 * SPDX-License-Identifier: Apache-2.0
 * 
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *   http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 *
 * See the License for the specific language governing permissions and
 * limitations under the License.
 *
 * @summary short description for the file
****************************************************************************/

And headers which includes other licenced code, we could use following

/****************************************************************************
 *  Copyright (c) 2018, <AUTHOR>
 *  All rights reserved.
 *
 *  Redistribution and use in source and binary forms, with or without
 *  modification, are permitted provided that the following conditions are met:
 *      * Redistributions of source code must retain the above copyright
 *        notice, this list of conditions and the following disclaimer.
 *      * Redistributions in binary form must reproduce the above copyright
 *        notice, this list of conditions and the following disclaimer in the
 *        documentation and/or other materials provided with the distribution.
 *      * Neither the name of the Sensirion AG nor the names of its
 *        contributors may be used to endorse or promote products derived
 *        from this software without specific prior written permission.
 *
 *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
 *  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 *  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 *  ARE DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
 *  DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
 *  (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
 *  LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 *  ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 *  THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 *
 * @license 
 * Modified work Copyright (c) 2020 Pez-Globo and the Pufferfish project contributors
 * SPDX-License-Identifier: Apache-2.0
 * 
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *   http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 *
 * See the License for the specific language governing permissions and
 * limitations under the License.
 *
 * @summary short description for the file
****************************************************************************/
ethanjli commented 4 years ago

This looks good to me, @Sudhir-dev. Just to abstract from the template for files which include third-party code, we will need to replace the license text for the third-party code from a placeholder. I've also made some suggested edits to take advantage of JSDoc features; so I am proposing:

Javascript Files

Regular files:

/**
 * @summary A short one-line description for the file
 *
 * @file More detailed description for the file, if necessary;
 * perhaps spanning multiple lines.
 *
 * @copyright Pez-Globo and the Pufferfish project contributors
 *
 * @license Apache-2.0
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *   http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
 * either express or implied.
 *
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

And for files including third-party works

/**
 * @summary A short one-line description for the file
 *
 * @file More detailed description for the file, if necessary;
 * perhaps spanning multiple lines.
 *
 * @copyright Pez-Globo and the Pufferfish project contributors
 *
 * @license Apache-2.0
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *   http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
 * either express or implied.
 *
 * See the License for the specific language governing permissions and
 * limitations under the License.
 *
 * Describe the inclusion, use, and modifications of third-party works
 * in this file.
 * Third-party works included in this file are licensed as follows:
 *
 * Copyright (c) <THIRD-PARTY WORK 1's COPYRIGHT NOTICE>
 * <THIRD-PARTY WORK 1's LICENSE NOTICE>
 *
 * Copyright (c) <THIRD-PARTY WORK 2's COPYRIGHT NOTICE>
 * <THIRD-PARTY WORK 2's LICENSE NOTICE>
 */

I'm also suggesting to reduce the number of opening and closing asterisks, for consistency with JSDoc's examples and for maintanability. Do you have any thoughts about the changes I've drafted based on your draft?

Python Files

@Tanmay06 Do you have any thoughts about the following draft of module file header templates, which keep the license boilerplate out of the docstring used by Sphinx: Regular files (note that the docstring here is taken directly from the Google Python Style Guide, which we are following):

"""A one line summary of the module or program, terminated by a period.

Leave one blank line.  The rest of this docstring should contain an
overall description of the module or program.  Optionally, it may also
contain a brief description of exported classes and functions and/or usage
examples.

Typical usage example:

    foo = ClassFoo()
    bar = foo.FunctionBar()

"""
# Copyright (c) Pez-Globo and the Pufferfish project contributors
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#  http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
# either express or implied.
#
# See the License for the specific language governing permissions and
# limitations under the License.

And for files including third-party works:

"""A one line summary of the module or program, terminated by a period.

Leave one blank line.  The rest of this docstring should contain an
overall description of the module or program.  Optionally, it may also
contain a brief description of exported classes and functions and/or usage
examples.

Typical usage example:

    foo = ClassFoo()
    bar = foo.FunctionBar()

"""
# Copyright (c) Pez-Globo and the Pufferfish project contributors
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#  http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
# either express or implied.
#
# See the License for the specific language governing permissions and
# limitations under the License.

# Describe the inclusion, use, and modifications of third-party works
# in this file.
# Third-party works included in this file are licensed as follows:
#
# Copyright (c) <THIRD-PARTY WORK 1's COPYRIGHT NOTICE>
# <THIRD-PARTY WORK 1's LICENSE NOTICE>
#
# Copyright (c) <THIRD-PARTY WORK 2's COPYRIGHT NOTICE>
# <THIRD-PARTY WORK 2's LICENSE NOTICE>

C++ Files

@RLaxmanrao Do you have any thoughts about the following draft of file header templates (where the licensing boilerplate is intentionally kept out of the doxygen comment), as well as whether it conforms to Doxygen's rules? Regular files:

/// \file
/// \brief A short one-line description of the file.
///
/// More detailed description for the file, if necessary;
/// perhaps spanning multiple lines.

// Copyright (c) Pez-Globo and the Pufferfish project contributors
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//  http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
// either express or implied.
//
// See the License for the specific language governing permissions and
// limitations under the License.

And for files including third-party works:

/// \file
/// \brief A short one-line description of the file.
///
/// More detailed description for the file, if necessary;
/// perhaps spanning multiple lines.

// Copyright (c) Pez-Globo and the Pufferfish project contributors
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//  http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
// either express or implied.
//
// See the License for the specific language governing permissions and
// limitations under the License.

// Describe the inclusion, use, and modifications of third-party works
// in this file.
// Third-party works included in this file are licensed as follows:
//
// Copyright (c) <THIRD-PARTY WORK 1's COPYRIGHT NOTICE>
// <THIRD-PARTY WORK 1's LICENSE NOTICE>
//
// Copyright (c) <THIRD-PARTY WORK 2's COPYRIGHT NOTICE>
// <THIRD-PARTY WORK 2's LICENSE NOTICE>

Update: fixed the typo pointed out by Laxman in the comment below; and removed the year from the copyright line.

RLaxmanrao commented 4 years ago

@ethanjli This was looks fine for me, small change in sentence “// Third-party works included this file are licensed as follows:” to be changed as… “// Third-party works included in this file are licensed as follows:”