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.

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

ci: remove unnecessary steps & cleanup #118

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

okineadev
Copy link
Contributor

No description provided.

@okineadev okineadev marked this pull request as draft February 4, 2025 07:00
@okineadev okineadev marked this pull request as ready for review February 4, 2025 07:03
Copy link
Collaborator

@xhyrom xhyrom left a comment

Choose a reason for hiding this comment

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

remove all unnecessary changes - keep quotes, remove emojis.

i agree with removing gh cli installation and inlining the run step if there's only one command

@xhyrom xhyrom changed the title refactor(ci): refactor GitHub Actions for better readablility ci: remove unnecessary steps Feb 4, 2025
@xhyrom xhyrom changed the title ci: remove unnecessary steps ci: remove unnecessary steps & cleanup Feb 4, 2025
@okineadev
Copy link
Contributor Author

remove all unnecessary changes - keep quotes, remove emojis.

i agree with removing gh cli installation and inlining the run step if there's only one command

Why keep the quotes?

I'm against this, I want them to be only where they are needed

@okineadev
Copy link
Contributor Author

Regarding emojis - I want them to be at least in the Workflow name, because then it's much easier to navigate them

@okineadev
Copy link
Contributor Author

Personally, I use them in step names because it's easier for me to navigate, but if you stick to the emoji-free style, then it's ok

@xhyrom
Copy link
Collaborator

xhyrom commented Feb 4, 2025

remove all unnecessary changes - keep quotes, remove emojis.
i agree with removing gh cli installation and inlining the run step if there's only one command

Why keep the quotes?

I'm against this, I want them to be only where they are needed

I would keep them everywhere for consistency.

Regarding emojis - I want them to be at least in the Workflow name, because then it's much easier to navigate them

I don't see any reason for having emojis in the workflow name; it doesn't improve navigation.

@okineadev
Copy link
Contributor Author

okineadev commented Feb 4, 2025

I would keep them everywhere for consistency.

Then please - add them if you so want them

@okineadev
Copy link
Contributor Author

I don't understand why you didn't do it earlier

@okineadev
Copy link
Contributor Author

Let's better decide what to do with this PR, when you merge it, then you can add those quotes, because it is not my responsibility


Regarding emojis in the names of Workflows, here is a visual example of their use:

image

Thanks to them, it is much easier and more intuitive to understand what it does

@xhyrom
Copy link
Collaborator

xhyrom commented Feb 4, 2025

Okay, we can keep your changes but we should also change name of the autofix.ci workflow to make it consistent with others. Any idea?

@okineadev
Copy link
Contributor Author

okineadev commented Feb 4, 2025

Okay, we can keep your changes but we should also change name of the autofix.ci workflow to make it consistent with others. Any idea?

I see that it is written there that it must be specifically called as autofix.ci to work

There are two options:

  1. Do not change its name
  2. Make it so that it itself make commits on behalf of github-actions[bot] independently of autofix.ci and for transparency in general (this is what I prefer)

@xhyrom
Copy link
Collaborator

xhyrom commented Feb 4, 2025

Okay, we can keep your changes but we should also change name of the autofix.ci workflow to make it consistent with others. Any idea?

I see that it is written there that it must be specifically called as autofix.ci to work

There are two options:

  1. Do not change its name
  2. Make it so that it itself make commits on behalf of github-actions[bot] independently of autofix.ci and for transparency in general (this is what I prefer)

Oh, you're right! Well we can do the second thing but probably in different pull request since it's out of scope.

@okineadev
Copy link
Contributor Author

Okay, we can keep your changes but we should also change name of the autofix.ci workflow to make it consistent with others. Any idea?

I see that it is written there that it must be specifically called as autofix.ci to work
There are two options:

  1. Do not change its name
  2. Make it so that it itself make commits on behalf of github-actions[bot] independently of autofix.ci and for transparency in general (this is what I prefer)

Oh, you're right! Well we can do the second thing but probably in different pull request since it's out of scope.

Ok 👌

@xhyrom xhyrom merged commit 54cb141 into oven-sh:main Feb 4, 2025
35 of 50 checks passed
Copy link
Collaborator

@xhyrom xhyrom left a comment

Choose a reason for hiding this comment

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

thanks

@okineadev okineadev deleted the refactor branch February 4, 2025 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants