8000 [core] Cover cpplint for ray/src/ray/stats by 400Ping · Pull Request #50678 · ray-project/ray · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[core] Cover cpplint for ray/src/ray/stats #50678

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
Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

[core] Cover cpplint for ray/src/ray/stats #50678

wants to merge 7 commits into from

Conversation

400Ping
Copy link
Contributor
@400Ping 400Ping commented Feb 18, 2025

Why are these changes needed?

Ensure all .h and .cc files in src/ray/stats comply with cpplint rules.

Related issue number

Closes: #50636

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@400Ping 400Ping requested a review from a team as a code owner February 18, 2025 03:26
dentiny
dentiny reviewed Feb 18, 2025
View reviewed changes
edoakes
edoakes approved these changes Feb 18, 2025
View reviewed changes
Copy link
Collaborator
@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

Thanks!

@edoakes
Copy link
Collaborator
edoakes commented Feb 18, 2025

Looks like there's a build error: https://buildkite.com/ray-project/microcheck/builds/11576#01951922-3928-41c3-8543-c6bf95d05c12/186-978

dentiny
dentiny reviewed Feb 18, 2025
View reviewed changes
@jcotant1 jcotant1 added the core Issues that should be addressed in Ray Core label Feb 19, 2025
dentiny
dentiny reviewed Feb 19, 2025
View reviewed changes
src/ray/stats/tag_defs.h Outdated
@@ -11,6 +11,8 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include <string>
Copy link
Contributor

Choose a reason for hiding this comment

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

No, #pragma once first, otherwise these two header files are included anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

should include <utility> and opencensus/tags/tag_key.h

@400Ping
Copy link
Contributor Author
400Ping commented Feb 20, 2025 •
edited
Loading

Not sure how to fix this, I tried to build raylet to see where the problem is and my laptop crashed🥲

@dentiny
Copy link
Contributor
dentiny commented Feb 23, 2025 •
edited
Loading 

[2025-02-19T12:09:58Z] bazel-out/k8-opt/bin/python/ray/_raylet.cpp:148578: note: '-Wmisleading-indentation' is disabled from this point onwards, since column-tracking was disabled due to the size of the code/headers
--
  | [2025-02-19T12:09:58Z] 148578 \|     __pyx_v_language = ((struct __pyx_obj_3ray_7_raylet_Language *)values[0]);
  | [2025-02-19T12:09:58Z]        \|

@400Ping it's a known issue and should be fixed in main, could you please rebase and retry?

@400Ping
[core] Cover cpplint for ray/src/ray/stats
f210ffd 
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping
[chore] Add NOLINT to supress cpplint
1e49807 
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping
[chore] Fix literals and add to pre-commit
7831c5f 
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping
[chore] refactor tag_defs.h
8407def 
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping
[chore] Refactor linter
5093332 
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping
[chore] add macos and linux
5f1b6c6 
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping 8000 400Ping force-pushed the core/cover-cpplint-stats branch from c429acb to 5f1b6c6 Compare February 24, 2025 10:38
@400Ping 400Ping marked this pull request as draft February 25, 2025 13:52
@400Ping
[Fix] tag_defs linter
fd0a960 
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@hainesmichaelc hainesmichaelc added the community-contribution Contributed by the community label Apr 4, 2025
@dayshah
Copy link
Contributor
dayshah commented Apr 29, 2025

Hey @400Ping same comment as #51551, let me know if you want to keep working on this, and if you have any questions. Thanks!!

@dayshah dayshah self-requested a review April 29, 2025 22:07
@dayshah dayshah self-assigned this Apr 29, 2025
@400Ping
Copy link
Contributor Author
400Ping commented Apr 29, 2025

yes, I will continue to work on this.

@github-actions GitHub Actions
Copy link
github-actions bot commented Jun 6, 2025

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

@github-actions github-actions bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Jun 6, 2025
@400Ping
Copy link
Contributor Author
400Ping commented Jun 6, 2025

working on it

@github-actions github-actions bot removed the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Jun 7, 2025
@github-actions GitHub Actions
Copy link

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

@github-actions github-actions bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Jun 22, 2025
@400Ping
Copy link
Contributor Author
400Ping commented Jun 22, 2025

Will be working on it.

@github-actions github-actions bot removed the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dentiny dentiny dentiny left review comments

@edoakes edoakes edoakes approved these changes

@dayshah dayshah Awaiting requested review from dayshah

Assignees

@dayshah dayshah

Labels
community-contribution Contributed by the community core Issues that should be addressed in Ray Core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[core] Cover cpplint for ray/src/ray/stats
6 participants
@400Ping @edoakes @dentiny @dayshah @hainesmichaelc @jcotant1
0