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

ci: setup CI using script #29

Merged
merged 19 commits into from
Jul 6, 2023
Merged

ci: setup CI using script #29

merged 19 commits into from
Jul 6, 2023

Conversation

moriarty
Copy link
Contributor

@moriarty moriarty commented Jun 20, 2023 •
edited
Loading

These will likely fail due to serial not being released for ROS2

I don't want to squash merge this PR

@moriarty
ci: setup CI using script
ef68501 
- Run the same script that packages in ros-controls github org use
- https://rtw.stoglrobotics.de/master/use-cases/ros_packages/configure_repository.html

These will likely fail due to serial not being released for ROS2
- see #21
- wjwwood/serial#204
- wjwwood/serial#283

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
Copy link
Contributor Author

Note this PR is entirely auto-generated. I'm expecting it to fail, and will fix it up accordingly

@moriarty
serial: Add serial from tylerjw fork
4722c30 
- we would prefer not to use the fork but we need to turn on CI
- when wjwwood/serial#204 or
  wjwwood/serial#283 are closed we can switch

- related to #21

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
Copy link
Contributor Author

@abake48 or @MarqRazz I don't want to squash merge this PR because I want to easily revert specific commits in the future assuming either serial is released or there is a pure cmake version that we can depend on

@moriarty
run ament_uncrustify
90ea352 
This is a formatting autofix

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
reorder package.xml for validation
3dce97a 
move author to after maintainer

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
add serial fork & branch to all .repos files
7c1b550 
serial is not yet release in ros2 so it is needed here for CI jobs

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
ci: remove rhel github jobs
2e56e0e 
Removing RHEL jobs for now will first focus on releasing for Ubuntu

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
ci: remove abi compatibility jobs
1d95302 
Removing these jobs for now until we have finished release

Signed-off-by: Alex Moriarty <[email protected]>
MarqRazz
MarqRazz reviewed Jun 27, 2023
View reviewed changes
Copy link
Contributor

@MarqRazz MarqRazz left a comment

Choose a reason for hiding this comment

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

Is the goal to have all test pass before we merge this PR?

.github/workflows/ci-format.yml Show resolved Hide resolved
robotiq_driver/src/robotiq_gripper_interface.cpp Outdated Show resolved Hide resolved
.github/workflows/README.md Outdated Show resolved Hide resolved
@moriarty
pre-commit run --all-files
7565379 
Run pre-commit

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
Sync .clang-format with ament_lint
c7a2de7 
and run pre-commit all files

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
disable uncrustify: conflicts with clang format
801ce61 
This disables uncrustify we use clang format

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
Add missing copywrite notices
f139b2c 
- fixes copywrite linter complaint that these were missing

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
IWYU fixes
4715e75 
Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
flake8: apply fixes
f3216fe 
Signed-off-by: Alex Moriarty <[email protected]>
@codecov
Copy link

codecov bot commented Jul 3, 2023 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (main@39df060). Click here to learn what that means.
The diff coverage is n/a.

@@          Coverage Diff           @@
##             main     #29   +/-   ##
======================================
  Coverage        ?   0.00%           
======================================
  Files           ?       5           
  Lines           ?     352           
  Branches        ?       0           
======================================
  Hits            ?       0           
  Misses          ?     352           
  Partials        ?       0
Flag Coverage Δ
unittests 0.00% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@moriarty
Copy link
Contributor Author

moriarty commented Jul 3, 2023

Is the goal to have all test pass before we merge this PR?

Yes... I didn't expect this PR to take so long.

@moriarty
Revert "flake8: apply fixes"
f0427d9 
This reverts commit f3216fe.
@moriarty
Add flake8 to pre-commit and add .flake8
825b7d3 
ament_lint runs flake8 and flake8 and black are conflicting over " vs '

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
fix linter include order
eac5fcc 
Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
clang/cpplint include " vs <
199a0d4 
With < clang-format will re-order
When re-ordered cpplint complains

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty
disable ament_flake8 flake8 via pre-commit
3b3f368 
- disable ament_flake8, flake8 via pre-commit reads .flake8 config
- ament_flake8 in ros-tooling didn't read .flake8
- flake8 and black will conflit over single vs double quotes

Signed-off-by: Alex Moriarty <[email protected]>
@moriarty moriarty requested a review from MarqRazz July 3, 2023 04:53
@moriarty moriarty requested a review from livanov93 July 5, 2023 07:55
MarqRazz
MarqRazz approved these changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@MarqRazz MarqRazz left a comment

Choose a reason for hiding this comment

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

Do we expect each ros release to need a different .repos file? Would be nice if we could squash that down to a single file. This PR adds so many configuration files (but a lot of tests too!).

@moriarty moriarty merged commit bba8578 into main Jul 6, 2023
20 checks passed
@delete-merged-branch delete-merged-branch bot deleted the setup-ci-with-script branch July 6, 2023 15:00
@moriarty
Copy link
Contributor Author

moriarty commented Jul 6, 2023

Do we expect each ros release to need a different .repos file? Would be nice if we could squash that down to a single file. This PR adds so many configuration files (but a lot of tests too!).

Basically the reason for each repo to have a uniq repos file is that they could require different branches...

@destogl has a script which sets this all up, and is what is used for the ROS 2 Control related repos.

I do agree that it pollutes the top directory with a bunch of stuff so maybe I re-organize and move them into a subdir

@moriarty moriarty mentioned this pull request Feb 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarqRazz MarqRazz MarqRazz approved these changes

@abake48 abake48 Awaiting requested review from abake48

@eholum eholum Awaiting requested review from eholum

@livanov93 livanov93 Awaiting requested review from livanov93

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@moriarty @MarqRazz