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

Update references to LTS from v1.6 to v1.10 #56729

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

PallHaraldsson
Copy link
Contributor

@PallHaraldsson PallHaraldsson commented Dec 1, 2024

No description provided.

@giordano
Copy link
Contributor

giordano commented Dec 2, 2024

The only result of using [skip ci] is skipping the mandatory checks, making a PR unmergeable. Also, it's not clear to me what unwanted or by whom.

@PallHaraldsson PallHaraldsson marked this pull request as draft December 2, 2024 00:22
@ViralBShah
Copy link
Member

ViralBShah commented Dec 2, 2024

This is just updating the LTS version in CONTRIBUTING.md, which doesn't affect any builds. I think this is ok to merge.

The issue title is confusing.

@nsajko nsajko added the docs This change adds or pertains to documentation label Dec 2, 2024
@giordano
Copy link
Contributor

giordano commented Dec 2, 2024

This is just updating the LTS version in CONTRIBUTING.md, which doesn't affect any builds.

What I was trying to say is that technically [skip ci] skips only GitHub Actions jobs, not the buildkite ones (don't ask me why) and the github actions jobs are those marked as required to run successfully for a PR to be merged, if they're skipped there's no merge button at all, with the result that (a) CI time isn't significantly saved, quite the opposite, you'll have to run it twice, and (b) the PR can't be merged even if it was good.

@PallHaraldsson PallHaraldsson marked this pull request as ready for review December 2, 2024 08:04
@giordano
Copy link
Contributor

giordano commented Dec 2, 2024

@PallHaraldsson also, if it wasn't clear, the problem is your commit message, not the first messageof the PR which is not used by anything. You have to change your commit to remove [skip ci]. Also, you've used [skip ci] several times before, someone might hope you'd have realised by now that [skip ci] with buildkite doesn't achieve anything. Finally, please make the title more meaningful, I still don't know what is unwanted, that's a very strange title.

@PallHaraldsson
Copy link
Contributor Author

you'd have realised by now that [skip ci] with buildkite doesn't achieve anything.

I didn't, I thought useful for docs, maybe only in the past, but will now stop doing this (I suppose neither helpful in packages).

I meant anything related to 1.6, backporting to it is not wanted. This was supposed to be a minor trivial PR... :) I see from Viral "I think this is ok to merge." Is as is better, to point to 1.10, since LTS, or 1.11? I'm always just using the web interfaced for PRs... so not sure how easy to change but I can change the title at least, to whatever you suggest.

@giordano
Copy link
Contributor

giordano commented Dec 2, 2024

I didn't, I thought useful for docs, maybe only in the past, but will now stop doing this (I suppose neither helpful in packages).

As I said, buildkite just ignores [skip ci] and it's used only by GitHub Actions, but we also have required GHA jobs, so we can't skip them at all. Summary: don't use [skip ci].

I meant anything related to 1.6, backporting to it is not wanted. This was supposed to be a minor trivial PR... :)

Just write something clear and objective like "update references to LTS from v1.6 to v1.10", don't add needless opinions. This all distracts from the main point of a proposed change, causes needless discussion and delays (and using [skip ci] just makes it impossible to merge anything, as I've been trying to repeat several times now)

@PallHaraldsson PallHaraldsson changed the title Not wanted since 1.6 no longer LTS. Update references to LTS from v1.6 to v1.10 Dec 2, 2024
@KristofferC
Copy link
Member

I updated the commit message and removed the ci skip from it.

@ViralBShah ViralBShah merged commit ba8290e into JuliaLang:master Dec 3, 2024
7 checks passed
@PallHaraldsson PallHaraldsson deleted the patch-32 branch December 3, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants