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

Updated regex and adding test cases for copyright search #363

Merged
merged 7 commits into from
Jun 3, 2022
Merged

Updated regex and adding test cases for copyright search #363

merged 7 commits into from
Jun 3, 2022

Conversation

zflat
Copy link
Contributor

@zflat zflat commented Mar 23, 2022 •
edited
Loading

This branch updates the copyright search to support different forms of capitalization and adds tests cases for different combinations of capitalization. See discussion at: #359

William Wedler added 3 commits March 24, 2022 14:50
adding test cases for copyright search
7bba2dd 
Signed-off-by: William Wedler <william.wedler@resquared.com>
added more test cases for capitalization
1fb3783 
Signed-off-by: William Wedler <william.wedler@resquared.com>
optional capture groups for alternative caplitalizations
80d3741 
Signed-off-by: William Wedler <william.wedler@resquared.com>
@zflat zflat force-pushed the improved_copyright_search branch from 0b6d6fe to 80d3741 Compare March 24, 2022 18:50
@zflat zflat marked this pull request as ready for review March 24, 2022 18:53
@zflat zflat changed the title adding test cases for copyright search Updated regex and adding test cases for copyright search Mar 24, 2022
@zflat zflat mentioned this pull request Mar 24, 2022
Closed
audrow
audrow approved these changes Mar 31, 2022
View reviewed changes
Copy link
Contributor

@audrow audrow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A few nitpicks, but looks good to me with green CI.

Here's a run of CI to check if things break downstream:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

ament_copyright/ament_copyright/parser.py Outdated Show resolved Hide resolved
ament_copyright/test/test_parser.py Outdated Show resolved Hide resolved
William Wedler added 4 commits March 31, 2022 00:11
more specific test assertions
46f8842 
Signed-off-by: William Wedler <william.wedler@resquared.com>
Revert "optional capture groups for alternative caplitalizations"
2593f40 
This reverts commit 80d3741.

Signed-off-by: William Wedler <william.wedler@resquared.com>
make copyright search case insensitive
6b94d31 
Signed-off-by: William Wedler <william.wedler@resquared.com>
added test case for multiple copyrights
ffa84db 
Signed-off-by: William Wedler <william.wedler@resquared.com>
This was referenced Mar 31, 2022
Merged
Merged
audrow
audrow approved these changes Mar 31, 2022
View reviewed changes
Copy link
Contributor

@audrow audrow left a comment

Choose a reason for hiding this comment

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

Looks good. Here's another round of CI to check after the changes:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@zflat
Copy link
Contributor Author

zflat commented Apr 5, 2022

@audrow Is this ready to be merged in?

@audrow
Copy link
Contributor

audrow commented Apr 5, 2022 •
edited
Loading

Unfortunately, I think this will have to wait two weeks while rolling is in a feature freeze to prepare for the upcoming Humble release. Sorry for the delay, the last week has been very busy preparing for the freeze.

@audrow
Copy link
Contributor

audrow commented Apr 27, 2022 •
edited
Loading

Alright, rolling is unfrozen, so I've kicked off another round of CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@audrow
Copy link
Contributor

audrow commented Apr 28, 2022

I think CI is down for maintenance. I'll run CI again in the near future.

@audrow
Copy link
Contributor

audrow commented May 2, 2022

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@zflat
Copy link
Contributor Author

zflat commented May 10, 2022

@audrow Did CI fail due to CMake Error at /home/jenkins-agent/workspace/ci_linux/ws/build/orocos_kdl/orocos_kdl-config.cmake:9 (find_package) unrelated to changes in this branch? If so, can CI be run again?

@zflat
Copy link
Contributor Author

zflat commented Jun 2, 2022

@methylDragon Can I get a new CI run and this merged?

@methylDragon
Copy link
Contributor

@methylDragon Can I get a new CI run and this merged?

Sure thing! Let's give this a try:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@methylDragon
Copy link
Contributor

Mm, I think there's an issue with the repos file that was used in CI.

Let me spin up my own, give me a sec!

@methylDragon
Copy link
Contributor

One more time:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@zflat
Copy link
Contributor Author

zflat commented Jun 3, 2022

@methylDragon Thanks for helping with this! However, I am not sure if the correct branch was built in CI? There should be test results for the newly added tests like in this previous job https://ci.ros2.org/job/ci_linux/16494/testReport/ament_copyright.test/test_parser/

The .repos file used in the test should point to my repo and the branch as the version key. See the repo file from before ( https://gist.githubusercontent.com/audrow/1d5c87e8169027dac9460e79a13df2b5/raw/fc82926ce7cdd9f7403ff37dd3a7993d1353544c/ros2.repos) with this entry:

  ament/ament_lint:
    type: git
    url: https://github.com/zflat/ament_lint.git
    version: improved_copyright_search

@methylDragon
Copy link
Contributor

methylDragon commented Jun 3, 2022 •
edited
Loading

Ah, whoops, I used zflat:improved_copyright_search instead of improved_copyright_search

You'll see it in the branch override on the CI

>>> branch: zflat:improved_copyright_search, <<<
use_connextdds: true,
use_connext_debs: false,
use_cyclonedds: true,
use_fastrtps_static: true,
use_fastrtps_dynamic: false,

...

Thanks for pointing this out! Let me do a rebuild

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@methylDragon
Copy link
Contributor

methylDragon commented Jun 3, 2022 •
edited
Loading

Looks like we're good to go!

CH3EERS!

@methylDragon methylDragon merged commit ae672fd into ament:master Jun 3, 2022
@zflat zflat deleted the improved_copyright_search branch June 9, 2022 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@audrow audrow audrow approved these changes

Assignees

@audrow audrow

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zflat @audrow @methylDragon