Skip to content

Remove the deprecated clustertask command#2523

Merged
tekton-robot merged 1 commit intotektoncd:mainfrom
pratap0007:remove-dep-ct
May 19, 2025
Merged

Remove the deprecated clustertask command#2523
tekton-robot merged 1 commit intotektoncd:mainfrom
pratap0007:remove-dep-ct

Conversation

@pratap0007
Copy link
Contributor

@pratap0007 pratap0007 commented Mar 24, 2025 •
edited
Loading

Clustertask support is deprecated and removed
this patch will remove the clustertask support

closes: #2485

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes tests (if functionality changed/added)
  • Run the code checkers with make check
  • Regenerate the manpages, docs and go formatting with make generated
  • Commit messages follow commit message best practices

See the contribution guide
for more details.

Release Notes

Remove the deprecated clustertask command

@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 24, 2025
@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 24, 2025
@pratap0007
Copy link
Contributor Author

pratap0007 commented Mar 24, 2025

/hold

@tekton-robot tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 24, 2025
@pratap0007 pratap0007 force-pushed the remove-dep-ct branch 2 times, most recently from 8db8b71 to dd1f8bd Compare April 3, 2025 14:24
@pratap0007
Copy link
Contributor Author

pratap0007 commented Apr 14, 2025

/retest

@vdemeester
Copy link
Member

vdemeester commented Apr 14, 2025

Seems like the error is the following

 test/README.md
	ERROR	https://yamllint.readthedocs.io/en/stable/index.html
		Forbidden (HTTP error 403)

@pratap0007
Copy link
Contributor Author

pratap0007 commented Apr 15, 2025

Seems like the error is the following

 test/README.md
	ERROR	https://yamllint.readthedocs.io/en/stable/index.html
		Forbidden (HTTP error 403)

I didn't find the reason of this error but link is accessible

@pratap0007
Copy link
Contributor Author

pratap0007 commented Apr 22, 2025

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 22, 2025
@pratap0007 pratap0007 force-pushed the remove-dep-ct branch 2 times, most recently from 5189bae to 4b829b3 Compare April 22, 2025 15:47
@vdemeester
Copy link
Member

vdemeester commented Apr 25, 2025

/retest

@pratap0007 pratap0007 force-pushed the remove-dep-ct branch 2 times, most recently from 44d6de4 to b9a938d Compare April 28, 2025 06:50
aThorp96
aThorp96 reviewed Apr 30, 2025
View reviewed changes
pkg/cmd/task/create.go
Copy link
Member

@aThorp96 aThorp96 Apr 30, 2025

Choose a reason for hiding this comment

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

This file is entirely deleted, but the documentation indicates that task create is still a valid command in this PR. Were tkn task commands deleted intentionally here or are the docs incorrect?

Copy link
Contributor

@piyush-garg piyush-garg May 6, 2025

Choose a reason for hiding this comment

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

hey @aThorp96

i think the task create has only functionality to create task from clustertask. I think we dont need this command now anymore. @pratap0007 can you please fix the doc accordingly. Thanks

Copy link
Contributor Author

@pratap0007 pratap0007 May 6, 2025

Choose a reason for hiding this comment

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

Yes, It has only functionality to create task from clustertask and I update the doc.

@pratap0007
Copy link
Contributor Author

pratap0007 commented May 6, 2025

@aThorp96 @piyush-garg Thanks for review, I have updated and can you please review it again?

aThorp96
aThorp96 approved these changes May 7, 2025
View reviewed changes
Copy link
Member

@aThorp96 aThorp96 left a comment

Choose a reason for hiding this comment

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

/lgtm

Some non-functional requests/cleanup

test/README.md Outdated
### Prerequisite to run make check and make generated

Ensure that golangci-lint and yamllint are installed on your development machine. If not then install them following [golangci-lint](https://golangci-lint.run/usage/install/) and [yamllint](https://yamllint.readthedocs.io/en/stable/quickstart.html).
Ensure that golangci-lint and yamllint are installed on your development machine. If not then install them following [golangci-lint](https://golangci-lint.run/welcome/install/) and yamllint.
Copy link
Member

@aThorp96 aThorp96 May 7, 2025

Choose a reason for hiding this comment

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

Any reason we're removing the link to yamllint?

Copy link
Contributor Author

@pratap0007 pratap0007 May 8, 2025

Choose a reason for hiding this comment

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

Markdown build test for URL link in readme was failing and will fix and the link in a separate PR

Copy link
Member

@aThorp96 aThorp96 May 8, 2025

Choose a reason for hiding this comment

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

After a little searching I believe that error could be due to some combination of three things:

  1. The tool which tektoncd/plubing uses to lint for dead links, lyche, was marked as deprecated in December of 2020. This should probably be addressed, but separate from this PR of course.
  2. It's not unheard of for Read The Docs to occasionally serve 403s to CI jobs intermittently in general.
  3. Given how out of date the linter is, and the exponential rise in AI crawlers (and in turn sites like Read The Docs having to take protective measures against those crawlers), if the linter if failing consistently for this link I suspect there may be scraping-protection preventing the linter from reaching the site.

Given the above, would you object to linking to the yamllint repo, https://github.com/adrienverge/yamllint/, instead or its readthedocs page? There are at least three projects named yamllint and it would be helpful IMO for the documentation to specify which yamllint we use.

Copy link
Contributor Author

@pratap0007 pratap0007 May 8, 2025

Choose a reason for hiding this comment

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

@aThorp96 Thanks a lot for taking a look at it, and I update it.

@tekton-robot
Copy link
Contributor

tekton-robot commented May 7, 2025

@aThorp96: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm

Some non-functional requests/cleanup

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@pratap0007
Remove the deprecated clustertask command
0bcc83e 
Clustertask support is deprecated and removed
this patch will remove the clustertask support

Signed-off-by: Shiv Verma <[email protected]>
@pratap0007
Copy link
Contributor Author

pratap0007 commented May 14, 2025

Thanks @aThorp96 for review, @vinamra28 @piyush-garg @divyansh42 can you please review?

divyansh42
divyansh42 approved these changes May 14, 2025
View reviewed changes
Copy link
Member

@divyansh42 divyansh42 left a comment

Choose a reason for hiding this comment

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

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 14, 2025
vinamra28
vinamra28 approved these changes May 19, 2025
View reviewed changes
Copy link
Member

@vinamra28 vinamra28 left a comment

Choose a reason for hiding this comment

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

/lgtm

@tekton-robot
Copy link
Contributor

tekton-robot commented May 19, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aThorp96, divyansh42, vinamra28

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2025
@tekton-robot tekton-robot merged commit 260211c into tektoncd:main May 19, 2025
8 checks passed
@divyansh42 divyansh42 mentioned this pull request May 19, 2025
Merged
4 tasks
@divyansh42 divyansh42 mentioned this pull request May 27, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@divyansh42 divyansh42 divyansh42 approved these changes

@vinamra28 vinamra28 vinamra28 approved these changes

@vdemeester vdemeester Awaiting requested review from vdemeester

+2 more reviewers

@piyush-garg piyush-garg piyush-garg left review comments

@aThorp96 aThorp96 aThorp96 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@divyansh42 divyansh42

@vinamra28 vinamra28

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove the cluster task command

7 participants

@pratap0007 @vdemeester @tekton-robot @piyush-garg @divyansh42 @vinamra28 @aThorp96