✨ Allow Downstream Root Commands to add custom descriptions #1830

anmol372 · 2020-11-17T17:39:28Z

Added 4 Options to configure command descriptions.
Downstream Projects may want to change the preset descriptions and examples to something more relevant to their projects so I am proposing the following Options to configure those by calling
WithRootCommandConfig(cmdCfg)
The fields of cmdCfg can be set by using RootCommandConfig struct added in the cli pkg

k8s-ci-robot · 2020-11-17T17:39:37Z

Welcome @anmol372!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-11-17T17:39:37Z

Hi @anmol372. 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.

Adirio

I'm not sure if a single Option that sets the three of them is the best approach. I feel that we want one option per field. So I would say either just one Option per field or one Option per field and a forth one that sets the three at once.

pkg/cli/cli.go

Adirio · 2020-11-17T17:53:51Z

/ok-to-test

anmol372 · 2020-11-17T22:08:16Z

@Adirio, I have gone with the four Option's approach and reverted the default descriptions to their pre tab formatting.

Adirio

/lgtm

You can resolve the conversations to collapse them.

Adirio · 2020-11-18T06:16:09Z

Retesting the failing test (it itmed out and the limit has been raised to 20 from the previous 15 minutes, which used to be enough but it depends on the workload of prow from several repositories). I expect it to pass it.
/retest

About the coverage decrease, could you add tests for these new options? Check how the WithCommandName test is performed in pkg/cli/cli_test.go and create a new context for the new provided Options that check equivalent fields.

kubebuilder/pkg/cli/cli_test.go

Lines 280 to 292 in 9663fb3

    
           Context("WithCommandName", func() { 
        
           	It("should use the provided command name", func() { 
        
           		commandName := "other-command" 
        
           		c, err = New( 
        
           			WithCommandName(commandName), 
        
           			WithDefaultPlugins(pluginAV1), 
        
           			WithPlugins(allPlugins...), 
        
           		) 
        
           		Expect(err).NotTo(HaveOccurred()) 
        
           		Expect(c).NotTo(BeNil()) 
        
           		Expect(c.(*cli).commandName).To(Equal(commandName)) 
        
           	}) 
        
           })

After you add the tests, we can ping a member with aproval privileges and merge this.

camilamacedo86 · 2020-11-18T17:01:00Z

Hi @anmol372,

The new methods need to be converted with unit tests. See: coverage/coveralls — Coverage decreased (-0.7%) to 71.827% and https://coveralls.io/builds/35041416/source?filename=pkg/cli/cli.go.

k8s-ci-robot · 2020-11-18T20:50:21Z

New changes are detected. LGTM label has been removed.

Adirio · 2020-11-18T21:50:35Z

/retest

estroz

There's probably a more generic/extensible way to do this like

type cli struct {
-	// Root command name. Can be injected downstream.
-	commandName string
-	// Root command short description, Can be injected downstream.
-	short string
-	// Root command long description, Can be injected downstream.
-	long string
-	//Root command example, Can be injected downstream.
-	example string
+   // cmdCfg configures the root cobra.Command.
+   cmdCfg RootCommandConfig

    ...
}

type RootCommandConfig struct {
    CommandName string
    Short string
    Long string
    Example string
}

// WithRootCommandConfig configures a CLI's root command.
func WithRootCommandConfig(cfg RootCommandConfig) Option {
	return func(c *cli) error {
		c.cmdCfg = cfg
		return nil
	}
}

func (c cli) defaultCommand() *cobra.Command {
    cmd := &cobra.Command{}

	// Prefer the WithCommandName() option for backwards-compat.
	if c.commandName != "" {
		cmd.Use = c.commandName
	} else {
        cmd.Use = c.cmdCfg.CommandName
    }
    cmd.Short = c.cmdCfg.Short
	if cmd.Short == "" {
		cmd.Short = "Development kit for building ..."
    }
    cmd.Long = c.cmdCfg.Long
	if cmd.Long == "" {
		cmd.Long = fmt.Sprintf(`Development kit for building ...`)
	}
    cmd.Example = c.cmdCfg.Example
	if cmd.Example == "" {
		cmd.Example = fmt.Sprintf(`...`)
	}

    return cmd
}

A solve-all solution would set type RootCommandConfig cobra.Command and use that as the base root command, but that may expose too much of the underlying CLI code and make users think they can set certain fields that will actually be overwritten/modified by other bits of the cli package.

Adirio · 2020-11-20T19:56:37Z

A solve-all solution would set type RootCommandConfig cobra.Command and use that as the base root command, but that may expose too much of the underlying CLI code and make users think they can set certain fields that will actually be overwritten/modified by other bits of the cli package.

I think that not showing that we are using cobra underneath as much as we could was a way to allow to remove that dependency in a future in case we wanted to.

Related to the RootCommandConfig struct, I think that using the Option declarative pattern, adding another layer of indirection would not solve anything and just adds complexity, but thats just personal preference.

anmol372 · 2020-11-22T02:16:59Z

Hi @estroz , @Adirio ,
I have updated the code as per review suggestions and I removed the WithCommandName() option completely, since the package has not been released uptill now, this seemed like a cleaner approach if we do add RootCommandConfig struct indirection and it would allow us to add more cobra attributes if/when required.
Please let me know if you think this is OK.

Adirio

There are multiple places (like each command descriptions or contexts) where you are using c.cmdCfg.CommandName when it could be empty. The only place where you are considering that it may be empty is when assigning it to c.cmd.Use. By providing a default full c.cmdCfg and making the option only updated the non-empty ones you ensure that it is always non-empty. The following changes are all about this except for one of them which just removed the quotes around a variable.

pkg/cli/cli.go

designs/extensible-cli-and-scaffolding-plugins-phase-1.md

pkg/cli/cli_test.go

pkg/cli/cli.go

anmol372 · 2020-11-23T15:20:48Z

Hi @Adirio,
Good Catch, I don't know how I missed that I'm setting c.cmd and using c.cmdCfg everywhere.
Thank You, I'm updating the PR for this.

camilamacedo86

The commits require to be squashed for we are able o get this one merged here. Could you please do it?

anmol372 · 2020-11-24T18:32:49Z

First one thing I forgot to tell you: removing WithCommandName is a breaking change. You probably don't want this to be a breaking change. Offering a way of setting the 4 in a single Option seems reasonable but I expect to have ways to set each of them by their own too.

I realize that removing WithCommandName would be a breaking change, but I was under the impression that the cli package hadn't been released yet.

Imagine the following case:
c := cli.New(
	WithCommandName("other-command-name"),
	WithRootCommandConfig(RootCommandConfig{
		Short: "Descritpion for other-command-name",
		Examples: `Examples for other-command-name`,
	}),
)
Now your command name is blank and will default to kubebuilder but the descriptions and examples are set for the other command. I think it is preferably to set the defaults, then parse the options to set the final values, and then template whatever is needed.

This scenario is the reason I choose to remove WithCommandName after the PR switched to using the RootCommandConfig struct. Because providing multiple ways to set the same value, did not make sense to me.

I did say that, but I changed my mind, sorry. I didn't know which implications it had by then as the PR has evolved quite a bit since then. Also I had to dive deep into the cli code recently, so that may have had something to do with the change. Again sorry for my previous suggestion.

That's reasonable, opinions will change over the course of a simple PR that is taking longer than expected, I guess. 😄

Yes, you are right, but proper documentation should be enough. It won't be a error, more of a warning, and it would be in the side of the command developer, not the final user. The command developer is expected to know the API of the cli library and if it requires a template with 1 param he should provide it.

While the developer is expected to know the API, it seems a tad bit unnecessary to allow them to set their own descriptions in their projects with the stipulation of using a single param.😅

Tailoring the Options to provide each of the descriptors separately may also lead to the expectation of not setting the descriptors that are not provided by the command developer?

Adirio · 2020-11-24T20:09:57Z

I realize that removing WithCommandName would be a breaking change, but I was under the impression that the cli package hadn't been released yet.

While there is no kubebuilder release, there are already projects like operator-SDK which are already using the library.

This scenario is the reason I choose to remove WithCommandName after the PR switched to using the RootCommandConfig struct. Because providing multiple ways to set the same value, did not make sense to me.

If you set the defaults at the start and make the WithRootCommandConfig Option to only set non-blank descriptors, that should be solved.

While the developer is expected to know the API, it seems a tad bit unnecessary to allow them to set their own descriptions in their projects with the stipulation of using a single param.😅

Maybe we can switch to a Replacer approach similar to how paths are templated. This will allow developers to provide %[command]s which will be replaced by the command name but it doesn't fail if you just provide a normal string.

replacer := strings.NewReplacer("%[command]s", c.cmdCfg.CommandName)
short := replacer.Replace(c.cmdCfg.Short)
long := replacer.Replace(c.cmdCfg.Long)
examples := replacer.Replace(c.cmdCfg.Examples)

You can do this any moment you want after calling the options in the loop.

Tailoring the Options to provide each of the descriptors separately may also lead to the expectation of not setting the descriptors that are not provided by the command developer?

Well if you want to solve this you would have to make the fields be pointers to strings, so that a nil pointer can be distinguised from the zero string ("") and only setting them when they are not nil.

estroz · 2020-11-24T21:28:56Z

While there is no kubebuilder release, there are already projects like operator-SDK which are already using the library.

Treating the main branch as the dev branch means breaking changes can happen between releases, which the operator-sdk team is fine with.

Maybe we can switch to a Replacer approach similar to how paths are templated

I like this to a degree, but wouldn't it be easier to require a command name be set?

In general, the CLI shouldn't make any assumptions about what is or isn't set: fields in RootCommandConfig should be used to directly set cobra.Command fields if not empty, otherwise use the default values (for which we know what needs to be substituted and where).

Adirio · 2020-11-24T22:51:05Z

Treating the main branch as the dev branch means breaking changes can happen between releases, which the operator-sdk team is fine with.

Right, but if a breaking change can be avoided, I would go for that. Also, I prefer to offer the possibility to set just one of those fields. Giving options to the user to a certain degree is nice IMO.

Maybe we can switch to a Replacer approach similar to how paths are templated

I like this to a degree, but wouldn't it be easier to require a command name be set?

In general, the CLI shouldn't make any assumptions about what is or isn't set: fields in RootCommandConfig should be used to directly set cobra.Command fields if not empty, otherwise use the default values (for which we know what needs to be substituted and where).

The issue is not the command name, @estroz, the issue is how to set it inside the description and examples. If we go for the fmt.Sprintf approach, when the user provides a decription without a single %s or any %[1]s, fmt.Sprintf will add a ugly warning to the string. On the other hard, using a strings.Replacer doesn't print any warning in any case but still gives the user the ability to set the command name, or us to set it in the default description and examples.

estroz · 2020-11-24T23:26:50Z

If we go for the fmt.Sprintf approach, when the user provides a decription without a single %s or any %[1]s, fmt.Sprintf will add a ugly warning to the string

If a description is provided, the description should be used verbatim, i.e. don't modify user-specified help text. The only case in which the CLI should format a string with command name is if a default string is used, because the CLI knows that formatting directives exist.

Adirio · 2020-11-25T05:22:04Z

If we go for the fmt.Sprintf approach, when the user provides a decription without a single %s or any %[1]s, fmt.Sprintf will add a ugly warning to the string

If a description is provided, the description should be used verbatim, i.e. don't modify user-specified help text. The only case in which the CLI should format a string with command name is if a default string is used, because the CLI knows that formatting directives exist.

Why special-case the default description to be fmted after calling the options (we have to wait until then to know the command name) and not provide this (already developed) capability to the user provided description? I agree that most of the time the user who provided the description will know the command name, and therefore, can provide the final description. And that would also allow to provide some alternative descriptors in the future. For example, a colored one.

anmol372 · 2020-11-25T19:29:38Z

/retest

Adirio · 2020-11-26T07:07:38Z

/retest

Adirio

A couple NITs

P.S.: you can mark all the conversations in #1830 (review) as resolved to hide them.

Adirio · 2020-11-26T07:12:12Z

pkg/cli/cli.go

@@ -113,10 +168,28 @@ func (c cli) Run() error {
 	return c.cmd.Execute()
 }

-// WithCommandName is an Option that sets the cli's root command name.
-func WithCommandName(name string) Option {
+// WithRootCommandConfig configures a CLI's root command.


CLI is the interface, Options are defined on cli, not on CLI.

Suggested change

// WithRootCommandConfig configures a CLI's root command.

// WithRootCommandConfig configures a cli's root command.

Adirio · 2020-11-26T07:20:27Z

pkg/cli/cli.go

+		if cfg.CommandName != "" {
+			c.cmdCfg.CommandName = cfg.CommandName
+		}
+		replacer := strings.NewReplacer(defaultCommandName, c.cmdCfg.CommandName)
+		if cfg.Short != "" {
+			c.cmdCfg.Short = cfg.Short
+		} else {
+			c.cmdCfg.Short = replacer.Replace(defaultShortDescription)
+		}
+		if cfg.Long != "" {
+			c.cmdCfg.Long = cfg.Long
+		} else {
+			c.cmdCfg.Long = replacer.Replace(defaultLongDescription)
+		}
+		if cfg.Example != "" {
+			c.cmdCfg.Example = cfg.Example
+		} else {
+			c.cmdCfg.Example = replacer.Replace(defaultExample)
+		}


So basically only default descriptors get replaced, user-provided descriptors are expected to know the command name. I'm OK with this, it simplifies a bit the API for cli developers as we don't have to tell them that they have the %[command]s placeholder but it still uses the correct command name in the default descriptors in case a different command name is provided.

NIT: we may want to stick to something different that kubebuilder as the placeholder (even if the commnad name is kubebuilder, the default descriptors will replace it by itself) as that would allow us to use the actual string "kubebuilder" in comments, e.g. to provide urls. Tests would also need to be updated.

Adirio · 2020-11-26T07:23:35Z

pkg/cli/cli.go

@@ -390,50 +463,10 @@ func (c cli) buildRootCmd() *cobra.Command {
 // defaultCommand returns the root command without its subcommands.
 func (c cli) defaultCommand() *cobra.Command {


Being now a direct return, only a couple lines long, and just being called from one place, I wonder if it makes sense to have it as a separate function or we could just create it in place. Also, defaultCommand does not make sense any more as the function name as previously it was providing default descriptors but now we can customize them and c.cdm already has the "final" descriptors.

estroz · 2020-12-08T19:30:22Z

Why special-case the default description to be fmted after calling the options (we have to wait until then to know the command name) and not provide this (already developed) capability to the user provided description

I'm fine not doing this. I'm really just trying to expose the simplest API possible: pass the CLI a struct containing help strings that will be used verbatim, with no changes. Increasing complexity with replacement semantics means more confusion for users of an already-complex plugin system.

estroz

/approve

k8s-ci-robot · 2020-12-18T17:11:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anmol372, estroz

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~~ [estroz]

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

k8s-ci-robot · 2021-01-20T19:28:14Z

@anmol372: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubebuilder-e2e-k8s-1-16-15	`df485ae`	link	`/test pull-kubebuilder-e2e-k8s-1-16-15`
pull-kubebuilder-e2e-k8s-1-14-10	`df485ae`	link	`/test pull-kubebuilder-e2e-k8s-1-14-10`
pull-kubebuilder-e2e-k8s-1-15-12	`df485ae`	link	`/test pull-kubebuilder-e2e-k8s-1-15-12`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-ci-robot · 2021-01-20T19:28:16Z

@anmol372: PR needs rebase.

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.

Adirio · 2021-01-20T23:13:13Z

Root command recently received a rework:

Short doesn't make sense for a root command as the short description is only showed when listing subcommands, and the root command is not a subcommand by definition.
Examples is being built dinamically based on the available plugins and supported project versions. This is important in order to provide useful error messages when unable to resolve plugins.

This means that, from the 3 configurable fields that this PR is adding, only Long makes sense to be added. And, being Long a single field, the multi-field in one config struct approach doesn't fit any more.

@anmol372 Do you want to update this PR just to include the Long description or do you want it to be done in a separate PR? In that case, do you want to take the task or should we raise an issue and marked it as help-wanted & good-first-issue so that other people develops it?

anmol372 · 2021-01-21T14:35:42Z

Hi @Adirio ,
I'll update this PR to reflect the Root command rework and add just the Long Description field.

Adirio

Request a review from (at least) me once you get this done.

camilamacedo86 · 2021-02-13T14:35:46Z

Hi @anmol372,

Could you please rebase it with the master for we are able to move forward here? Also, can you close all comments that are addressed?

camilamacedo86 · 2021-03-06T17:38:55Z

Hi @anmol372,

It is open for a while without be rebased with the master? Could you please rebase this one and let us know if you could address all suggestion and if it is done for another review round? OR is no longer it required?

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 17, 2020

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 17, 2020

k8s-ci-robot requested review from Adirio and droot November 17, 2020 17:39

Adirio suggested changes Nov 17, 2020

View reviewed changes

pkg/cli/cli.go Outdated Show resolved Hide resolved

pkg/cli/cli.go Outdated Show resolved Hide resolved

pkg/cli/cli.go Outdated Show resolved Hide resolved

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 Nov 17, 2020

anmol372 marked this pull request as ready for review November 17, 2020 22:11

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 17, 2020

Adirio approved these changes Nov 17, 2020

View reviewed changes

k8s-ci-robot assigned Adirio Nov 17, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2020

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2020

estroz suggested changes Nov 20, 2020

View reviewed changes

Adirio suggested changes Nov 22, 2020

View reviewed changes

camilamacedo86 reviewed Nov 23, 2020

View reviewed changes

anmol372 force-pushed the downstream_command_description branch from 45fc713 to a61d369 Compare November 23, 2020 15:54

Adirio reviewed Nov 26, 2020

View reviewed changes

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

anmol372 force-pushed the downstream_command_description branch from aafb81a to 86ac9d4 Compare December 16, 2020 16:06

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

Allow Downstream Root Commands to add custom descriptions

df485ae

anmol372 force-pushed the downstream_command_description branch from 86ac9d4 to df485ae Compare December 16, 2020 16:47

anmol372 mentioned this pull request Dec 16, 2020

Apply updates from operator-sdk helm-operator operator-framework/helm-operator-plugins#63

Open

10 tasks

anmol372 requested a review from Adirio December 18, 2020 15:02

estroz approved these changes Dec 18, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 18, 2020

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

Adirio suggested changes Jan 29, 2021

View reviewed changes

Adirio mentioned this pull request Mar 24, 2021

✨ Allow to modify the root's command description #2105

Merged

k8s-ci-robot closed this in #2105 Mar 24, 2021

	// WithRootCommandConfig configures a CLI's root command.
	// WithRootCommandConfig configures a cli's root command.

		@@ -390,50 +463,10 @@ func (c cli) buildRootCmd() *cobra.Command {
		// defaultCommand returns the root command without its subcommands.
		func (c cli) defaultCommand() *cobra.Command {

✨ Allow Downstream Root Commands to add custom descriptions #1830

✨ Allow Downstream Root Commands to add custom descriptions #1830

Conversation

anmol372 commented Nov 17, 2020 • edited Loading

k8s-ci-robot commented Nov 17, 2020

k8s-ci-robot commented Nov 17, 2020

Adirio left a comment

Choose a reason for hiding this comment

Adirio commented Nov 17, 2020

anmol372 commented Nov 17, 2020

Adirio left a comment

Choose a reason for hiding this comment

Adirio commented Nov 18, 2020 • edited Loading

camilamacedo86 commented Nov 18, 2020

k8s-ci-robot commented Nov 18, 2020

Adirio commented Nov 18, 2020

estroz left a comment • edited Loading

Choose a reason for hiding this comment

Adirio commented Nov 20, 2020 • edited Loading

anmol372 commented Nov 22, 2020 • edited Loading

Adirio left a comment

Choose a reason for hiding this comment

anmol372 commented Nov 23, 2020 • edited Loading

camilamacedo86 left a comment

Choose a reason for hiding this comment

anmol372 commented Nov 24, 2020 • edited Loading

Adirio commented Nov 24, 2020 • edited Loading

estroz commented Nov 24, 2020

Adirio commented Nov 24, 2020

estroz commented Nov 24, 2020

Adirio commented Nov 25, 2020

anmol372 commented Nov 25, 2020

Adirio commented Nov 26, 2020

Adirio left a comment • edited Loading

Choose a reason for hiding this comment

Adirio Nov 26, 2020

Choose a reason for hiding this comment

Adirio Nov 26, 2020

Choose a reason for hiding this comment

Adirio Nov 26, 2020

Choose a reason for hiding this comment

estroz commented Dec 8, 2020

estroz left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Dec 18, 2020

k8s-ci-robot commented Jan 20, 2021

k8s-ci-robot commented Jan 20, 2021

Adirio commented Jan 20, 2021

anmol372 commented Jan 21, 2021 • edited Loading

Adirio left a comment

Choose a reason for hiding this comment

camilamacedo86 commented Feb 13, 2021

camilamacedo86 commented Mar 6, 2021

anmol372 commented Nov 17, 2020 •

edited

Loading

Adirio commented Nov 18, 2020 •

edited

Loading

estroz left a comment •

edited

Loading

Adirio commented Nov 20, 2020 •

edited

Loading

anmol372 commented Nov 22, 2020 •

edited

Loading

anmol372 commented Nov 23, 2020 •

edited

Loading

anmol372 commented Nov 24, 2020 •

edited

Loading

Adirio commented Nov 24, 2020 •

edited

Loading

Adirio left a comment •

edited

Loading

anmol372 commented Jan 21, 2021 •

edited

Loading