You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@weex.apache.org by 王仁敏 <wr...@gmail.com> on 2019/08/06 07:03:38 UTC
[Discuss] Recent works about CI
Recently I spent some time on Travis CI, the below is the progress：
1. Added Android Lint check and OCLint check in Travis CI.
-
[Android Lint]
If the files in the android folder are modified, new-created, or deleted,
the AndroidLint check will be triggered. Once any rules of AndroidLint are
triggered, Travis CI will build failed and the result of AndroidLint will
be displayed at the PR. (We have solved all the problems reported by
AndroidLint)
-
[OCLint]
If the C++ or Objective-C files are modified, new-created, or deleted, the
OCLint check will be triggered. OCLint is the same as Android Lint, Once
any rules of OCLint are triggered, Travis CI will build failed and the
result of OCLint will be displayed at the PR. (We are solving all the
problems reported by OCLint and will complete soon.)
But there are too many rules of OCLint and many of them have a little
effect on our project. So we disable some of the rules, as the is shown.
2. Add Check of the iOS project.
Only ios-sdk project build successful and all the test cases of the
ios-playground pass, the check will succeed. otherwise, The Travis CI will
build failed.
3. Add code-format validation check.
use Clang-Format to validate the code format, as Clang-Format supports code
format of the llvm languages, such as c++, objective-c, java, etc.
I found a danger plugin named danger-code_style_validation
<https://github.com/flix-tech/danger-code_style_validation> almost
satisfied our needs. it uses 'clang-format' to look for code style
violations in added lines on the current PR and offers inline patches.
To meet our needs, I fork that repo and I did some change based on it :
1.
change message level from error to warn(because I think the code format
is recommended and not necessary.)
2.
modify the message hint and make it more readable.
In Travis CI, the Weex project will download this plugin through a GitHub
link and use it to validate code-format. here is the configuration
<https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11>
.
but I am not sure whether this behavior meets the requirements of AFS, If
not, what should I do?
Links:
OCLint diable rule:
https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/.travis.yml#L167
Re: [Discuss] Recent works about CI
Posted by 王仁敏 <wr...@gmail.com>.
You are right, only code format task in my PR is relied on third-party
code.I have removed the code of code format task. So you can merge the PR
now.
Thanks for your advice.
York Shen <sh...@gmail.com> 于2019年8月7日周三 下午3:56写道：
> Correct me if I am wrong.
>
> It looks like you are relying on third-party code only for code_format
> task in TravisCI. If this is the case, I suggest you remove code_format
> task in your PR first, as everything else looks good to me, I could just
> merge your PR at that time. Then we can still discuss how to solve the
> third party dependencies, as it’s a time consuming task according to my
> knowledge.
>
> > 在 2019年8月6日，16:10，申远 <sh...@gmail.com> 写道：
> >
> > First of all, thanks for your effort of improving Weex CI. I know
> debugging TravisCI is a painful experience and you have wrote over one
> hundred commits [1] to make TravisCI right.
> >
> > If this is about dependencies in source code, you should definitely copy
> those code into weex_sdk and change the LICENSE file to make third-party IP
> clear. As this is about dependencies in CI environment, and the
> corresponding code will never be bundled into the release of weex_sdk, I
> think forking others' repo is acceptable. Even so, I don't think it's ok
> that you relies on a certain branch of the forking repo [2]. At least you
> should publish a release version for your forking repo and relies on that
> version in weex_sdk.
> >
> > Maybe mentors here could give some advice on this issue as it's not the
> same source code dependencies issue that I am familiar with.
> >
> > @Jan @Myrle @Willem
> >
> > [1] https://github.com/apache/incubator-weex/pull/2731 <
> https://github.com/apache/incubator-weex/pull/2731>
> > [2]
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11
> <
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11
> >
> >
> > Best Regards,
> > YorkShen
> >
> > 申远
> >
> >
> > 王仁敏 <wrmwindmill@gmail.com <ma...@gmail.com>> 于2019年8月6日周二
> 下午3:04写道：
> > Recently I spent some time on Travis CI, the below is the progress：
> >
> > 1. Added Android Lint check and OCLint check in Travis CI.
> >
> > -
> >
> > [Android Lint]
> >
> > If the files in the android folder are modified, new-created, or deleted,
> > the AndroidLint check will be triggered. Once any rules of AndroidLint
> are
> > triggered, Travis CI will build failed and the result of AndroidLint will
> > be displayed at the PR. (We have solved all the problems reported by
> > AndroidLint)
> >
> > -
> >
> > [OCLint]
> >
> > If the C++ or Objective-C files are modified, new-created, or deleted,
> the
> > OCLint check will be triggered. OCLint is the same as Android Lint, Once
> > any rules of OCLint are triggered, Travis CI will build failed and the
> > result of OCLint will be displayed at the PR. (We are solving all the
> > problems reported by OCLint and will complete soon.)
> >
> > But there are too many rules of OCLint and many of them have a little
> > effect on our project. So we disable some of the rules, as the is shown.
> >
> > 2. Add Check of the iOS project.
> >
> > Only ios-sdk project build successful and all the test cases of the
> > ios-playground pass, the check will succeed. otherwise, The Travis CI
> will
> > build failed.
> >
> > 3. Add code-format validation check.
> >
> > use Clang-Format to validate the code format, as Clang-Format supports
> code
> > format of the llvm languages, such as c++, objective-c, java, etc.
> >
> > I found a danger plugin named danger-code_style_validation
> > <https://github.com/flix-tech/danger-code_style_validation <
> https://github.com/flix-tech/danger-code_style_validation>> almost
> > satisfied our needs. it uses 'clang-format' to look for code style
> > violations in added lines on the current PR and offers inline patches.
> >
> > To meet our needs, I fork that repo and I did some change based on it :
> >
> > 1.
> >
> > change message level from error to warn(because I think the code
> format
> > is recommended and not necessary.)
> > 2.
> >
> > modify the message hint and make it more readable.
> >
> > In Travis CI, the Weex project will download this plugin through a GitHub
> > link and use it to validate code-format. here is the configuration
> > <
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11
> <
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11
> >>
> > .
> >
> > but I am not sure whether this behavior meets the requirements of AFS, If
> > not, what should I do?
> >
> >
> > Links:
> >
> > OCLint diable rule:
> >
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/.travis.yml#L167
> <
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/.travis.yml#L167
> >
>
>
Re: [Discuss] Recent works about CI
Posted by 申远 <sh...@gmail.com>.
Thanks, both of you.
In this case, I suggest we solve the problem under following procedure:
1. Remove the dependencies of danger-code_style_validation, then I can
merge the PR.
2. Publish it to somewhere( npm or other places ) with a different name
and a new version, then create a pull request where you rely the new
dependency you just published.
Best Regards,
YorkShen
申远
王仁敏 <wr...@gmail.com> 于2019年8月8日周四 上午10:02写道：
> >
> > Thanks for your detailed reply.
> >
> > The license of the code itself is fine in my case. I think publish it to
> > npm with a different name maybe a better solution. I will spend some time
> > to do it.
> >
>
Re: [Discuss] Recent works about CI
Posted by 王仁敏 <wr...@gmail.com>.
>
> Thanks for your detailed reply.
>
> The license of the code itself is fine in my case. I think publish it to
> npm with a different name maybe a better solution. I will spend some time
> to do it.
>
Re: [Discuss] Recent works about CI
Posted by Jan Piotrowski <pi...@gmail.com>.
As far as I am aware ASF rules only apply to code included in the
releases - what you do to build or test your code is not really
relevant to these requirements.
But obviously you should follow the licences of the code itself, which
seems fine in this case.
Of course if you want to keep that in the codebase longer term, the
repo should probably become part of the apache org. But for small
things like that it might be fine to keep it in your personal org, and
maybe just publish it to npm with a different name.
J
Am Mi., 7. Aug. 2019 um 09:56 Uhr schrieb York Shen <sh...@gmail.com>:
>
> Correct me if I am wrong.
>
> It looks like you are relying on third-party code only for code_format task in TravisCI. If this is the case, I suggest you remove code_format task in your PR first, as everything else looks good to me, I could just merge your PR at that time. Then we can still discuss how to solve the third party dependencies, as it’s a time consuming task according to my knowledge.
>
> > 在 2019年8月6日，16:10，申远 <sh...@gmail.com> 写道：
> >
> > First of all, thanks for your effort of improving Weex CI. I know debugging TravisCI is a painful experience and you have wrote over one hundred commits [1] to make TravisCI right.
> >
> > If this is about dependencies in source code, you should definitely copy those code into weex_sdk and change the LICENSE file to make third-party IP clear. As this is about dependencies in CI environment, and the corresponding code will never be bundled into the release of weex_sdk, I think forking others' repo is acceptable. Even so, I don't think it's ok that you relies on a certain branch of the forking repo [2]. At least you should publish a release version for your forking repo and relies on that version in weex_sdk.
> >
> > Maybe mentors here could give some advice on this issue as it's not the same source code dependencies issue that I am familiar with.
> >
> > @Jan @Myrle @Willem
> >
> > [1] https://github.com/apache/incubator-weex/pull/2731 <https://github.com/apache/incubator-weex/pull/2731>
> > [2] https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11 <https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11>
> >
> > Best Regards,
> > YorkShen
> >
> > 申远
> >
> >
> > 王仁敏 <wrmwindmill@gmail.com <ma...@gmail.com>> 于2019年8月6日周二 下午3:04写道：
> > Recently I spent some time on Travis CI, the below is the progress：
> >
> > 1. Added Android Lint check and OCLint check in Travis CI.
> >
> > -
> >
> > [Android Lint]
> >
> > If the files in the android folder are modified, new-created, or deleted,
> > the AndroidLint check will be triggered. Once any rules of AndroidLint are
> > triggered, Travis CI will build failed and the result of AndroidLint will
> > be displayed at the PR. (We have solved all the problems reported by
> > AndroidLint)
> >
> > -
> >
> > [OCLint]
> >
> > If the C++ or Objective-C files are modified, new-created, or deleted, the
> > OCLint check will be triggered. OCLint is the same as Android Lint, Once
> > any rules of OCLint are triggered, Travis CI will build failed and the
> > result of OCLint will be displayed at the PR. (We are solving all the
> > problems reported by OCLint and will complete soon.)
> >
> > But there are too many rules of OCLint and many of them have a little
> > effect on our project. So we disable some of the rules, as the is shown.
> >
> > 2. Add Check of the iOS project.
> >
> > Only ios-sdk project build successful and all the test cases of the
> > ios-playground pass, the check will succeed. otherwise, The Travis CI will
> > build failed.
> >
> > 3. Add code-format validation check.
> >
> > use Clang-Format to validate the code format, as Clang-Format supports code
> > format of the llvm languages, such as c++, objective-c, java, etc.
> >
> > I found a danger plugin named danger-code_style_validation
> > <https://github.com/flix-tech/danger-code_style_validation <https://github.com/flix-tech/danger-code_style_validation>> almost
> > satisfied our needs. it uses 'clang-format' to look for code style
> > violations in added lines on the current PR and offers inline patches.
> >
> > To meet our needs, I fork that repo and I did some change based on it :
> >
> > 1.
> >
> > change message level from error to warn(because I think the code format
> > is recommended and not necessary.)
> > 2.
> >
> > modify the message hint and make it more readable.
> >
> > In Travis CI, the Weex project will download this plugin through a GitHub
> > link and use it to validate code-format. here is the configuration
> > <https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11 <https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11>>
> > .
> >
> > but I am not sure whether this behavior meets the requirements of AFS, If
> > not, what should I do?
> >
> >
> > Links:
> >
> > OCLint diable rule:
> > https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/.travis.yml#L167 <https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/.travis.yml#L167>
>
Re: [Discuss] Recent works about CI
Posted by York Shen <sh...@gmail.com>.
Correct me if I am wrong.
It looks like you are relying on third-party code only for code_format task in TravisCI. If this is the case, I suggest you remove code_format task in your PR first, as everything else looks good to me, I could just merge your PR at that time. Then we can still discuss how to solve the third party dependencies, as it’s a time consuming task according to my knowledge.
> 在 2019年8月6日，16:10，申远 <sh...@gmail.com> 写道：
>
> First of all, thanks for your effort of improving Weex CI. I know debugging TravisCI is a painful experience and you have wrote over one hundred commits [1] to make TravisCI right.
>
> If this is about dependencies in source code, you should definitely copy those code into weex_sdk and change the LICENSE file to make third-party IP clear. As this is about dependencies in CI environment, and the corresponding code will never be bundled into the release of weex_sdk, I think forking others' repo is acceptable. Even so, I don't think it's ok that you relies on a certain branch of the forking repo [2]. At least you should publish a release version for your forking repo and relies on that version in weex_sdk.
>
> Maybe mentors here could give some advice on this issue as it's not the same source code dependencies issue that I am familiar with.
>
> @Jan @Myrle @Willem
>
> [1] https://github.com/apache/incubator-weex/pull/2731 <https://github.com/apache/incubator-weex/pull/2731>
> [2] https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11 <https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11>
>
> Best Regards,
> YorkShen
>
> 申远
>
>
> 王仁敏 <wrmwindmill@gmail.com <ma...@gmail.com>> 于2019年8月6日周二 下午3:04写道：
> Recently I spent some time on Travis CI, the below is the progress：
>
> 1. Added Android Lint check and OCLint check in Travis CI.
>
> -
>
> [Android Lint]
>
> If the files in the android folder are modified, new-created, or deleted,
> the AndroidLint check will be triggered. Once any rules of AndroidLint are
> triggered, Travis CI will build failed and the result of AndroidLint will
> be displayed at the PR. (We have solved all the problems reported by
> AndroidLint)
>
> -
>
> [OCLint]
>
> If the C++ or Objective-C files are modified, new-created, or deleted, the
> OCLint check will be triggered. OCLint is the same as Android Lint, Once
> any rules of OCLint are triggered, Travis CI will build failed and the
> result of OCLint will be displayed at the PR. (We are solving all the
> problems reported by OCLint and will complete soon.)
>
> But there are too many rules of OCLint and many of them have a little
> effect on our project. So we disable some of the rules, as the is shown.
>
> 2. Add Check of the iOS project.
>
> Only ios-sdk project build successful and all the test cases of the
> ios-playground pass, the check will succeed. otherwise, The Travis CI will
> build failed.
>
> 3. Add code-format validation check.
>
> use Clang-Format to validate the code format, as Clang-Format supports code
> format of the llvm languages, such as c++, objective-c, java, etc.
>
> I found a danger plugin named danger-code_style_validation
> <https://github.com/flix-tech/danger-code_style_validation <https://github.com/flix-tech/danger-code_style_validation>> almost
> satisfied our needs. it uses 'clang-format' to look for code style
> violations in added lines on the current PR and offers inline patches.
>
> To meet our needs, I fork that repo and I did some change based on it :
>
> 1.
>
> change message level from error to warn(because I think the code format
> is recommended and not necessary.)
> 2.
>
> modify the message hint and make it more readable.
>
> In Travis CI, the Weex project will download this plugin through a GitHub
> link and use it to validate code-format. here is the configuration
> <https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11 <https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11>>
> .
>
> but I am not sure whether this behavior meets the requirements of AFS, If
> not, what should I do?
>
>
> Links:
>
> OCLint diable rule:
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/.travis.yml#L167 <https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/.travis.yml#L167>
Re: [Discuss] Recent works about CI
Posted by 申远 <sh...@gmail.com>.
First of all, thanks for your effort of improving Weex CI. I know debugging
TravisCI is a painful experience and you have wrote over one hundred
commits [1] to make TravisCI right.
If this is about dependencies in source code, you should definitely copy
those code into weex_sdk and change the LICENSE file to make third-party IP
clear. As this is about dependencies in CI environment, and the
corresponding code will never be bundled into the release of weex_sdk, I
think forking others' repo is acceptable. Even so, I don't think it's ok
that you relies on a certain branch of the forking repo [2]. At least you
should publish a release version for your forking repo and relies on that
version in weex_sdk.
Maybe mentors here could give some advice on this issue as it's not the
same source code dependencies issue that I am familiar with.
@Jan @Myrle @Willem
[1] https://github.com/apache/incubator-weex/pull/2731
[2]
https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11
Best Regards,
YorkShen
申远
王仁敏 <wr...@gmail.com> 于2019年8月6日周二 下午3:04写道：
> Recently I spent some time on Travis CI, the below is the progress：
>
> 1. Added Android Lint check and OCLint check in Travis CI.
>
> -
>
> [Android Lint]
>
> If the files in the android folder are modified, new-created, or deleted,
> the AndroidLint check will be triggered. Once any rules of AndroidLint are
> triggered, Travis CI will build failed and the result of AndroidLint will
> be displayed at the PR. (We have solved all the problems reported by
> AndroidLint)
>
> -
>
> [OCLint]
>
> If the C++ or Objective-C files are modified, new-created, or deleted, the
> OCLint check will be triggered. OCLint is the same as Android Lint, Once
> any rules of OCLint are triggered, Travis CI will build failed and the
> result of OCLint will be displayed at the PR. (We are solving all the
> problems reported by OCLint and will complete soon.)
>
> But there are too many rules of OCLint and many of them have a little
> effect on our project. So we disable some of the rules, as the is shown.
>
> 2. Add Check of the iOS project.
>
> Only ios-sdk project build successful and all the test cases of the
> ios-playground pass, the check will succeed. otherwise, The Travis CI will
> build failed.
>
> 3. Add code-format validation check.
>
> use Clang-Format to validate the code format, as Clang-Format supports code
> format of the llvm languages, such as c++, objective-c, java, etc.
>
> I found a danger plugin named danger-code_style_validation
> <https://github.com/flix-tech/danger-code_style_validation> almost
> satisfied our needs. it uses 'clang-format' to look for code style
> violations in added lines on the current PR and offers inline patches.
>
> To meet our needs, I fork that repo and I did some change based on it :
>
> 1.
>
> change message level from error to warn(because I think the code format
> is recommended and not necessary.)
> 2.
>
> modify the message hint and make it more readable.
>
> In Travis CI, the Weex project will download this plugin through a GitHub
> link and use it to validate code-format. here is the configuration
> <
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/Gemfile#L11
> >
> .
>
> but I am not sure whether this behavior meets the requirements of AFS, If
> not, what should I do?
>
>
> Links:
>
> OCLint diable rule:
>
> https://github.com/apache/incubator-weex/blob/79494d2027760d9d6ba6d8ae807c6e155896df08/.travis.yml#L167
>