Skip to content

Conversation

LDong-Arm
Copy link
Contributor

@LDong-Arm LDong-Arm commented Aug 20, 2020

Summary of changes

Blocks: #13430

Some of subdirectories contain code imported from other projects that should be excluded from style and spell checks. While we have .astyleignore for style checks, the same list should apply to spell checks too.

This PR

  • renames .astyleignore to .codecheckignore which is more generic
  • adds an extra argument to spell.sh to take a list of ignored paths
  • applies .codecheckignore to spell checks in Travis.

Note: Some directories that were not checked before (e.g. "connectivity") should have their spells checked and fixed in the future - we do have typos in our own code. Then we will be able to check them in CI.

Impact of changes

None.

Migration actions required

None.

Documentation

None.


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

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

Covered by Travis.


Reviewers

@ARMmbed/mbed-os-maintainers @hugueskamba @jamesbeyond


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Aug 20, 2020
@ciarmcom ciarmcom requested review from a team August 20, 2020 15:30
@ciarmcom
Copy link
Member

@LDong-Arm, thank you for your changes.
@ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

@LDong-Arm LDong-Arm changed the title DO NOT MERGE: use .astyleignore for list of spell-checked files Support ignore list for spell checks Aug 20, 2020
@LDong-Arm
Copy link
Contributor Author

@0xc0170 @adbridge Spell checks are blocking #13430 due to external code. It would be really helpful if you could please have a look at this proposal, thank!

@evedon FYI.

done < <(find "${1}" -type d -iname "*target*" -prune -o -name '*.h' -print)
# ${1}: directory to check
# ${2}: file containing a list of paths (regex) to exclude
done < <(find "${1}" -type d -iname "*target*" -prune -o -name '*.h' -print | grep -v -f "${2}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally the second argument should be optional, but I'm not sure how to do it after a pipe. Unless we rewrite this part in a different way.

@mergify mergify bot added needs: CI and removed needs: work labels Aug 20, 2020
@mbed-ci
Copy link

mbed-ci commented Aug 21, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-ARM ✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage ✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest ✔️

@0xc0170 0xc0170 merged commit 7829238 into ARMmbed:master Aug 21, 2020
@mergify mergify bot removed the ready for merge label Aug 21, 2020
@mbedmain mbedmain added release-version: 6.3.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants