-
Notifications
You must be signed in to change notification settings - Fork 683
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@yichoi I've updated README.md after you approved since we don't need to modify jerry-ext/CMakeLists.txt any more. |
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akosthekiss I've updated.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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]