ros2 / rosbag2_bag_v2

rosbag2 plugin for replaying ros1 version2 bag files
Apache License 2.0
24 stars 4 forks source link

Add GitHub Actions for linting #12

Closed zmichaels11 closed 4 years ago

zmichaels11 commented 4 years ago

Changes

Testing

Signed-off-by: Zachary Michaels zmichaels11@gmail.com

piraka9011 commented 4 years ago

Is there a way we can fix the CMakeLists.txt warning? Maybe by using an AUTHOR_WARNING instead?

zmichaels11 commented 4 years ago

Is there a way we can fix the CMakeLists.txt warning? Maybe by using an AUTHOR_WARNING instead?

I think that should be addressed in a different PR since those warnings are picked up by Jenkins and aren't influencing the added linters

zmichaels11 commented 4 years ago

@Karsten1987 can you take a look at this?

prajakta-gokhale commented 4 years ago

@Karsten1987 this is awaiting your review.

Karsten1987 commented 4 years ago

Taking the explanation of @thomas-moulard from here I am not sure how to interpret this PR as I only see the PR jobs and DCO on this. What exactly should I see here as checks?

piraka9011 commented 4 years ago

I am not sure how to interpret this PR as I only see the PR jobs and DCO on this. What exactly should I see here as checks?

It looks like because this is coming from a fork, the checks are not appearing on this PR. I've seen this happen before in other PRs (example).

Maybe try this out with a branch local to the repo? @zmichaels11

zmichaels11 commented 4 years ago

I have the same PR merging into a local branch here that runs the linters. They aren't running on this PR because the PR is coming from a fork.

Karsten1987 commented 4 years ago

@zmichaels11 can you open a PR from a local branch to see if that works? Also, how is this supposed to work in the future if non-members to this org unit want to open PRs? Then I would have to run the CI on these PRs manually again because the action wouldn't work?

zmichaels11 commented 4 years ago

@Karsten1987 It doesn't look like I can open a local branch in this repo. I think the linters would automatically run for forks once enabled on this repo.

I'll ask @thomas-moulard how he set it up for Rosbag2, since all of the PRs we're submitting to Rosbag2 are running the linters and are coming from a fork.

Karsten1987 commented 4 years ago

@zmichaels11 you can now push to this repo.

zmichaels11 commented 4 years ago

Closing in favor of local branch: https://github.com/ros2/rosbag2_bag_v2/pull/15