Skip to content

Change tizenrt-artik05x port reference board to artik053 #1904

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

Merged
merged 1 commit into from
Jul 19, 2017
Merged

Change tizenrt-artik05x port reference board to artik053 #1904

merged 1 commit into from
Jul 19, 2017

Conversation

glistening
Copy link
Contributor

Artik053 board is publicly announced about one month ago.
So I updated tizenrt-artik05x reference board from sidk_s5jt200 to artik053.
Now, tizenrt-artik05x port works with latest TizenRT repo on artik053 board.

Also, I replaced str_to_uint to strtol since #1891 is landed.

JerryScript-DCO-1.0-Signed-off-by: Sanggyu Lee [email protected]

Copy link
Contributor

@yichoi yichoi left a comment

Choose a reason for hiding this comment

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

LGTM

@glistening
Copy link
Contributor Author

@yichoi I've updated README.md after you approved since we don't need to modify jerry-ext/CMakeLists.txt any more.

@yichoi
Copy link
Contributor

yichoi commented Jul 5, 2017

Ok. still LGTM

# 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.
Copy link
Member

Choose a reason for hiding this comment

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

This is a strange copyright/license block. Don't we add all code under the "JS Foundation and other contributors" copyright only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akosthekiss At first, I added the JS foundation license since I thought all new/modified code in jerryscript project must have JS foundation license. Is it right? If not, could you give me the license rule? Anyway, for this case, I found that I did not modified Make.defs. (I modified defconfig only.) So I can remove the JS foundation license. Thank you for review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akosthekiss I've updated.

Copy link
Member

Choose a reason for hiding this comment

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

@glistening It seems that now the "JS Foundation and other contributors" copyright clause is gone and got replaced by company copyright notice. This should happen the other way around.

When the project was transferred to the Foundation, all "top-level" individual and company copyright notices were replaced by the catch-all "Copyright JS Foundation and other contributors" notice. (There are some files that have been imported into the project from external sources with non-Apache2 but compatible licenses. There, there appropriate notices are preserved below the Apache2 license block, see jerry-libm/*.c or jerry-core/ecma/base/ecma-helpers-errol.c. Thus the reference to "top-level" notices.) This has been documented in the Contribution Guidelines as well: https://github.com/jerryscript-project/jerryscript/blob/master/CONTRIBUTING.md#2-ensure-all-files-have-a-proper-license-header-and-copyright-notice

Any code that you want to contribute to the project must be licensed under the Apache License 2.0. Contributions under a different license can not be accepted.
Adding copyright notices other than the project-wide notice ("Copyright JS Foundation and other contributors, http://js.foundation") is not permitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akosthekiss I got it. You don't want to keep NuttX license in jerryscript project since the project only permits Apache2 license. Then, I have another question. What is the difference between this case and https://github.com/jerryscript-project/jerryscript/tree/master/third-party/valgrind/memcheck.h (or valgrind.h)? If these are Apache2 compatible licenses, NuttX seems to be also compatible when I take a close look into each clauses.

Copy link
Member

@akosthekiss akosthekiss Jul 17, 2017

Choose a reason for hiding this comment

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

@glistening Well, that's not exactly what I was trying to say. This commit tries to add an Apache 2.0 license header with a Copyright 2017 Samsung Electronics All Rights Reserved. notice. But that's explicitly prohibited by the contribution guidelines (as linked and quoted in my above comment). The Apache 2.0 license header must be accompanied by the Copyright JS Foundation and other contributors, http://js.foundation project-wide notice:

Adding copyright notices other than the project-wide notice ("Copyright JS Foundation and other contributors, http://js.foundation") is not permitted.

As for BSD or BSD-like licenses (e.g., the NuttX license): I did not want to suggest its removal (I think that would explicitly go against the provisions of BSD3.) Such a compatible license block (together with its copyright notice) can be kept below the Apache 2.0 license block (that's why I referred to jerry-libm and ecma-helpers-errol.c, which also contain an additional - although not BSD but other Apache 2.0 compatible - license block). Fortunately, this is permitted by our guidelines:

The only exception is adding third-party code which requires copyright notices to be preserved.

As for the third-party/valgrind library: I don't exactly know the history of that part. It's been there for a while and has not been touched since it's addition in 2014. I don't really know why it was not touched when the project was transferred to JS Foundation. However, I do think that any new code/contribution should follow the current guidelines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akosthekiss First, thank you again for explanation. By the way, I did not add the header with a Copyright 2017 Samsung Electronics All Rights Reserved. The header came from the base file in TizenRT. I just added JS Foundation license only. The result was my first commit like following:

JS Foundation license (Apache2 : for conforming jerryscript license rule)
Samsung license (Apache2 : from TizenRT)
NuttX license (BSD-like : from NuttX)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akosthekiss I've updated this PR like the above. Please give me comment if this PR needs further correction.

Copy link
Member

Choose a reason for hiding this comment

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

@glistening Ah, so. It was not evident to me from the original PR comment(s). Your explanation clarifies things, thanks.

Artik053 board is publicly announced about one month ago.
So I updated tizenrt-artik05x reference board from sidk_s5jt200 to artik053.
Now, tizenrt-artik05x port works with latest TizenRT repo on artik053 board.

Also, I replaced str_to_uint to strtol since #1891 is landed.

JerryScript-DCO-1.0-Signed-off-by: Sanggyu Lee [email protected]
Copy link
Member

@akosthekiss akosthekiss left a comment

Choose a reason for hiding this comment

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

LGTM

@yichoi yichoi merged commit 5fd98fa into jerryscript-project:master Jul 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants