coverage: 78.749% (+0.02%) from 78.726%
when pulling 0668ddb on gebamp:identity-v3-project-tag
into 22fe3c1 on gophercloud:master.

Hey, @mandre / @pierreprinetti could you possibly have a look at this? We sort of need it.

@mandre do we ever use an admin account of devstack in the acceptance tests? This could be a nice candidate

@mandre I did the Fixups, and included the acceptances tests!

@mandre Can you check this out, when you have time?

Apologies @gebamp, I'm traveling and won't be able to do a proper review. @pierreprinetti @EmilienM could you take a look?

please rebase and then LGTM

@EmilienM Done with the rebase, when you have time feel free to merge

since @pierreprinetti and @mandre were involved too, I'll let them a chance to a last review. Thanks for your work!

@EmilienM @mandre @pierreprinetti It has been a couple of months, since the last comment. Do we have any updates wrt to this MR?

There is a hold lock in check that prevents this PR to be merged. Any idea how to fix it?

Nope, I havent seen this before :)

I am adding a HOLD label to signify that squashing is needed before merging.

The only thing that is left is having the CheckTagResult option implement it's own Extract method.
If this is still required, I will go ahead and implement it adding new fixups. @pierreprinetti What is your opinion?

The only thing that is left is having the CheckTagResult option implement it's own Extract method. If this is still required, I will go ahead and implement it adding new fixups. @pierreprinetti What is your opinion?

I still think that CheckTagResult should have an Extract method. Do you think differently?

The only thing that is left is having the CheckTagResult option implement it's own Extract method. If this is still required, I will go ahead and implement it adding new fixups. @pierreprinetti What is your opinion?

I still think that CheckTagResult should have an Extract method. Do you think differently?

Since the user, can infer the result from the return codes of the error using the ExtractErr method, I dont think this is needed.
However, for this to happen we need to be strict and remove 404 from the list of ok codes of the CheckTag method.
Personally, I think it would be better to keep only 204 as the only success code to follow the api reference and not to include any Extract method for the CheckTagResult.

A link to the api reference for the checktag method:
https://docs.openstack.org/api-ref/identity/v3/#check-if-project-contains-tag

@pierreprinetti Any news wrt this?

The purpose of CheckTag is to tell whether the tag is present. The user shouldn't need to check the return code of the HTTP call to get their answer.

I don't think we want this code on the caller side:

	checkTagResult := projects.CheckTag(ctx, identityClient, "myproject", "mytag")
	if err := checkTagResult.ExtractErr(); err != nil {
		if checkTagResult.Result.StatusCode == 404 {
			fmt.Print("The tag does not exist")
		} else {
			panic(err)
		}
	} else {
		fmt.Print("The tag exists")
	}

Please implement an Extract() method on CheckTagResult that returns (bool, error) and leave both 204 and 404 as accepted codes, as they literally encode the answer to the question on that API endpoint.

	exists, err := projects.CheckTag(ctx, identityClient, "myproject", "mytag").Extract()
	if err != nil {
		panic(err)
	}

	if exists {
		fmt.Print("The tag exists")
	} else {
		fmt.Print("The tag does not exist")
	}

@pierreprinetti Thank you for the feedback, I will go ahead update the code based on the above requests!