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.

Sign up for GitHub

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

Jump to bottom

Importer script: remove apache-2.0.txt #12590

Merged
merged 2 commits into from
Mar 9, 2020
Merged

Importer script: remove apache-2.0.txt #12590

merged 2 commits into from
Mar 9, 2020

Conversation

urutva
Copy link
Contributor

@urutva urutva commented Mar 6, 2020

Summary of changes

Both Mbed TLS and Mbed Crypto libraries doesn't contain
apache-2.0.txt anymore. Do not access those files in the importer
script.

Signed-off-by: Devaraj Ranganna devaraj.ranganna@arm.com

Impact of changes

Migration actions required

Documentation

Pull request type 

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results 

[x] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@Patater

Patater
Patater previously approved these changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

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

Could you prefix your commit message title with "tls: "?

@urutva
tls: Remove apache-2.0.txt from importer script
4773a21 
Both Mbed TLS and Mbed Crypto libraries doesn't contain
`apache-2.0.txt` anymore. Do not access those files in the importer
script.

Signed-off-by: Devaraj Ranganna <devaraj.ranganna@arm.com>
@urutva urutva force-pushed the importer-remove-license-file branch from dafc70d to 4773a21 Compare March 6, 2020 11:27
@mergify mergify bot dismissed Patater’s stale review March 6, 2020 11:27

Pull request has been modified.

0xc0170
0xc0170 previously approved these changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

not valid file but not that we remove the license - I got afraid when I read the title at first

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 6, 2020

Could you prefix your commit message title with "tls: "?

Good one 💯

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 6, 2020

CI started

@urutva urutva changed the title Remove apache-2.0.txt from importer script Importer script: remove apache-2.0.txt Mar 6, 2020
@mergify mergify bot added needs: work and removed needs: CI labels Mar 6, 2020
simonbutcher
simonbutcher suggested changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@simonbutcher simonbutcher left a comment

Choose a reason for hiding this comment

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

I don't think this is sufficient @Devran01. The description of the licensing in the README.md also needs to be changed.

The text there says no CLA is required for trivial changes and for bigger changes we need a CLA signed. Both are wrong. This needs to be part of the update.

@Patater
Copy link
Contributor

Patater commented Mar 6, 2020

I don't think this is sufficient @Devran01. The description of the licensing in the README.md also needs to be changed.

The text there says no CLA is required for trivial changes and for bigger changes we need a CLA signed. Both are wrong. This needs to be part of the update.

This PR is about fixing our CI primarily, as the license file has gone missing in the source repos. This PR doesn't remove any license files, so this PR maintains the status quo, whilst fixing Mbed TLS CI.

Wrong text is what was in the Mbed TLS version we most recently pulled in to Mbed OS; and that should be fixed when we update Mbed TLS (as the README.md is sourced from there). The fix will come in naturally the next time we update the version of Mbed TLS we pull in to Mbed OS.

@mbed-ci
Copy link

mbed-ci commented Mar 6, 2020

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_cloud-client-pytest

@simonbutcher
Copy link
Contributor

simonbutcher commented Mar 6, 2020 •
edited
Loading

@Patater, this file README.md is not imported and is part of Mbed OS.

I'll submit a separate PR to fix the text.

@simonbutcher @urutva
Update the contribution guidelines for Mbed TLS
19b208f 
This commit changes the contribution guidelines to refer the user to the
contribution guidelines in the Mbed TLS repo. It also removes the outdated references to the
CLA, and CLA not being required for trivial changes.

Signed-off-by: Simon Butcher <simon.butcher@arm.com>
@mergify mergify bot dismissed 0xc0170’s stale review March 6, 2020 17:33

Pull request has been modified.

@ciarmcom ciarmcom requested review from Patater and a team March 6, 2020 18:00
@ciarmcom
Copy link
Member

ciarmcom commented Mar 6, 2020

@Devran01, thank you for your changes.
@Patater @ARMmbed/mbed-os-crypto @ARMmbed/mbed-os-tls @ARMmbed/mbed-os-maintainers please review.

Patater
Patater approved these changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

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

LGTM

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 9, 2020

Ci restarted

@mbed-ci
Copy link

mbed-ci commented Mar 9, 2020

Test run: SUCCESS

Summary: 7 of 7 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 merged commit e71ab0d into ARMmbed:master Mar 9, 2020
@mergify mergify bot removed the ready for merge label Mar 9, 2020
@urutva urutva deleted the importer-remove-license-file branch March 9, 2020 09:59
@Patater Patater mentioned this pull request Mar 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonbutcher simonbutcher simonbutcher requested changes

@Patater Patater Patater approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

Assignees
No one assigned
Labels
release-version: 6.0.0-alpha-3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@urutva @0xc0170 @Patater @mbed-ci @simonbutcher @ciarmcom