Skip to content

[ci] place all CI helpers under the .ci folder and use - instead of _ in their names#6581

Merged
jameslamb merged 4 commits intomasterfrom
ci/simplify-structure
Jul 31, 2024
Merged

[ci] place all CI helpers under the .ci folder and use - instead of _ in their names#6581
jameslamb merged 4 commits intomasterfrom
ci/simplify-structure

Conversation

@StrikerRUS
Copy link
Collaborator

Simplify repo's structure in the following way:

  1. Move all CI scripts under .ci folder
  2. use - instead of _ in scripts' names to ensure their correct ordering

breaking tag due to "public" build-r.R script.

Copy link
Collaborator

@borchero borchero left a comment

Choose a reason for hiding this comment

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

What's the benefit of this change? 😄

@StrikerRUS
Copy link
Collaborator Author

@borchero Simplification of the repo's structure. All helper scripts used in our CI are placed in one directory. Newcomers won't struggle deducing the difference between helpers and .ci folders, for example.

@borchero
Copy link
Collaborator

@StrikerRUS ok, I only briefly looked at the file changes before and it seemed like it would only include renames from snake case to kebab case. I see that it actually moves three files 😄

I guess (1) moving files makes a lot of sense, I'm not too sure about (2) changing the file names but I don't really mind.

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

I support the idea of removing helpers/ and just consolidating everything into ci/. That will definitely reduce a bit of friction.

I also support preferring - to _, as it requires one less keypress (it's a minor thing, but still nice!).

However.... I think we should keep build_r.R using snake_case. As you noted, it sort of is "public". It's the documented, preferred entry point for building custom variations of the R package (like with GPU support). I don't support breaking peoples' builds in exchange for a bit more consistency in the naming of scripts.

@StrikerRUS
Copy link
Collaborator Author

@jameslamb

I think we should keep build_r.R using snake_case.

Reverted in d0dbc63.

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

great, thank you

@jameslamb jameslamb merged commit 35a2a2e into master Jul 31, 2024
@jameslamb jameslamb deleted the ci/simplify-structure branch July 31, 2024 13:36
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2025

This pull request has been automatically locked since there has not been any recent activity since it was closed.
To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants