Load and calculate Destination of files for `kustomize localize` #4652

annasong20 · 2022-05-25T00:14:23Z

Implement locLoader, which loads and calculates destination paths of files for new command kustomize localize.

k8s-ci-robot · 2022-05-25T00:14:31Z

Hi @annasong20. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

api/loader/fileloader.go

k8s-ci-robot · 2022-05-25T00:59:47Z

@annasong20: This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

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.

annasong20 · 2022-05-25T01:01:56Z

/label tide/merge-method-squash

natasha41575 · 2022-05-25T23:43:38Z

/ok-to-test

natasha41575

Looks great! Mostly nits so far. I will do another pass after you update

api/loader/fileloader.go

api/localizer/writer.go

api/loader/fileloader_test.go

natasha41575 · 2022-05-26T00:23:26Z

api/localizer/writer.go

+	inNewDir, _ := filepath.Rel(scope.String(), target.String())
+	dst, dstErr := makeAndClean(newDir.Join(inNewDir), fSys)
+	if dstErr != nil {
+		return nil, errors.Wrapf(dstErr, "unable to write to newDir")


Suggested change

return nil, errors.Wrapf(dstErr, "unable to write to newDir")

return nil, errors.Wrapf(dstErr, fmt.Sprintf("unable to write to new directory %q", newDir)

%q will help to print something like unable to write to new directory 'newDir'

I'm leaning towards not printing newDir's path because it should already be included in dstErr, but nonetheless the fmt.Sprintf suggestion was super helpful.

I've re-written almost all of the error messages. Lmk if this is still a problem.

api/localizer/writer.go

api/internal/localizer/locloader_test.go

api/internal/localizer/locloader.go

natasha41575 · 2022-08-19T21:23:26Z

/lgtm
/approve

k8s-ci-robot · 2022-08-19T21:23:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: annasong20, natasha41575

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [natasha41575]

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

natasha41575 · 2022-08-19T21:26:42Z

/test all

Revert due to code freeze issue kubernetes-sigs#4756

…ze` (#4652)" This reverts commit bf17fe1.

Remove code that changes remotes-loading code path, as mandated by kubernetes-sigs#4756

Reinstate #4652 without url-related code

* Address TODO in PR #4652 * Improve readability

* Address TODO in PR kubernetes-sigs#4652 * Improve readability

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 25, 2022

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 25, 2022

k8s-ci-robot requested review from KnVerey and mortent May 25, 2022 00:14

annasong20 commented May 25, 2022

View reviewed changes

api/loader/fileloader.go Show resolved Hide resolved

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label May 25, 2022

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 25, 2022

natasha41575 reviewed May 26, 2022

View reviewed changes

api/localizer/writer.go Outdated Show resolved Hide resolved

natasha41575 mentioned this pull request Jun 1, 2022

fileloader_test: Replace lengthy t.Fatalf with require #4661

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2022

annasong20 marked this pull request as draft June 6, 2022 19:03

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 6, 2022

annasong20 force-pushed the localize-file branch from c503601 to 1095783 Compare June 6, 2022 19:19

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2022

annasong20 force-pushed the localize-file branch from 1095783 to 8d332fd Compare June 9, 2022 00:33

annasong20 mentioned this pull request Jun 14, 2022

Move demandDirectoryRoot into kyaml/filesys #4677

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2022

natasha41575 mentioned this pull request Jul 8, 2022

Clean code near Loader #4702

Closed

annasong20 changed the title ~~Implement file localization for localize~~ Load and calculate Destination of files for kustomize localize Jul 15, 2022

annasong20 force-pushed the localize-file branch from 8d332fd to 75905b6 Compare August 2, 2022 21:51

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2022

natasha41575 closed this Aug 2, 2022

natasha41575 reopened this Aug 2, 2022

natasha41575 reviewed Aug 17, 2022

View reviewed changes

api/internal/localizer/locloader_test.go Show resolved Hide resolved

Address code review round kubernetes-sigs#3

b902091

annasong20 commented Aug 18, 2022

View reviewed changes

api/internal/localizer/locloader.go Show resolved Hide resolved

natasha41575 reviewed Aug 18, 2022

View reviewed changes

api/internal/localizer/locloader.go Show resolved Hide resolved

api/internal/localizer/locloader.go Show resolved Hide resolved

Address code review feedback kubernetes-sigs#4

c9c491a

k8s-ci-robot assigned natasha41575 Aug 19, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 19, 2022

k8s-ci-robot merged commit bf17fe1 into kubernetes-sigs:master Aug 19, 2022

annasong20 deleted the localize-file branch August 19, 2022 22:08

annasong20 added a commit to annasong20/kustomize that referenced this pull request Aug 26, 2022

Revert remote url-related changes in kubernetes-sigs#4652

ebe76f1

Revert due to code freeze issue kubernetes-sigs#4756

annasong20 mentioned this pull request Aug 26, 2022

Revert remote url-related changes in #4652 #4774

Closed

natasha41575 added a commit that referenced this pull request Aug 26, 2022

Revert "Load and calculate Destination of files for `kustomize locali…

8868d91

…ze` (#4652)" This reverts commit bf17fe1.

natasha41575 mentioned this pull request Aug 26, 2022

Revert "Load and calculate Destination of files for kustomize localize" #4775

Merged

annasong20 added a commit to annasong20/kustomize that referenced this pull request Aug 29, 2022

Reinstate kubernetes-sigs#4652 without url-related code

6d99c8e

annasong20 added a commit to annasong20/kustomize that referenced this pull request Aug 29, 2022

Reinstate kubernetes-sigs#4652 without url-related code

f80cf9f

Remove code that changes remotes-loading code path, as mandated by kubernetes-sigs#4756

k8s-ci-robot added a commit that referenced this pull request Aug 30, 2022

Merge pull request #4779 from annasong20/locloader

cb26363

Reinstate #4652 without url-related code

natasha41575 mentioned this pull request Aug 30, 2022

Reinstate #4652 without url-related code #4779

Merged

This was referenced Sep 28, 2022

REQUEST: New membership for @annasong20 kubernetes/org#3736

Closed

repospec: support ssh urls with ssh certificates #4741

Closed

annasong20 mentioned this pull request Oct 25, 2022

Implement kustomize localize localization layer #4797

Closed

annasong20 added a commit to annasong20/kustomize that referenced this pull request Nov 4, 2022

Address TODO in PR kubernetes-sigs#4652

fd34463

annasong20 mentioned this pull request Nov 4, 2022

Address TODO in PR #4652 #4856

Merged

annasong20 added a commit to annasong20/kustomize that referenced this pull request Nov 5, 2022

Address TODO in PR kubernetes-sigs#4652

37fe6a7

annasong20 added a commit to annasong20/kustomize that referenced this pull request Nov 7, 2022

Address TODO in PR kubernetes-sigs#4652

08d1e96

k8s-ci-robot pushed a commit that referenced this pull request Nov 8, 2022

Address TODO in PR #4652 (#4856)

6d9b540

* Address TODO in PR #4652 * Improve readability

elisshafer pushed a commit to elisshafer/kustomize that referenced this pull request Dec 8, 2022

Address TODO in PR kubernetes-sigs#4652 (kubernetes-sigs#4856)

b32a0ac

* Address TODO in PR kubernetes-sigs#4652 * Improve readability

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load and calculate Destination of files for `kustomize localize` #4652

Load and calculate Destination of files for `kustomize localize` #4652

annasong20 commented May 25, 2022 •

edited

Loading

k8s-ci-robot commented May 25, 2022

k8s-ci-robot commented May 25, 2022

annasong20 commented May 25, 2022

natasha41575 commented May 25, 2022

natasha41575 left a comment

natasha41575 May 26, 2022 •

edited

Loading

annasong20 Jun 10, 2022 •

edited

Loading

annasong20 Aug 2, 2022

natasha41575 commented Aug 19, 2022

k8s-ci-robot commented Aug 19, 2022

natasha41575 commented Aug 19, 2022

	return nil, errors.Wrapf(dstErr, "unable to write to newDir")
	return nil, errors.Wrapf(dstErr, fmt.Sprintf("unable to write to new directory %q", newDir)

Load and calculate Destination of files for kustomize localize #4652

Load and calculate Destination of files for kustomize localize #4652

Conversation

annasong20 commented May 25, 2022 • edited Loading

k8s-ci-robot commented May 25, 2022

k8s-ci-robot commented May 25, 2022

annasong20 commented May 25, 2022

natasha41575 commented May 25, 2022

natasha41575 left a comment

Choose a reason for hiding this comment

natasha41575 May 26, 2022 • edited Loading

Choose a reason for hiding this comment

annasong20 Jun 10, 2022 • edited Loading

Choose a reason for hiding this comment

annasong20 Aug 2, 2022

Choose a reason for hiding this comment

natasha41575 commented Aug 19, 2022

k8s-ci-robot commented Aug 19, 2022

natasha41575 commented Aug 19, 2022

Load and calculate Destination of files for `kustomize localize` #4652

Load and calculate Destination of files for `kustomize localize` #4652

annasong20 commented May 25, 2022 •

edited

Loading

natasha41575 May 26, 2022 •

edited

Loading

annasong20 Jun 10, 2022 •

edited

Loading