Remove unused variable abort_incomplete_multipart_upload_days

[adamantike]

What

Remove variable abort_incomplete_multipart_upload_days.

Why

The abort_incomplete_multipart_upload_days variable isn't being used since the introduction of lifecycle_rules in #85.

Notes

closes #128

[korenyoni]

/test all

[Nuru]

@korenyoni Note to use as a reviewer: I think I might let it slide in this particular instance, but as a rule, when we find out a variable is no longer being used due to a recent change, I would prefer to re-enable the variable rather than remove it. We want to preserve backward compatibility as much as is reasonable.

Also, Please look at combining this with, or at least aligning with, #77 and #90.

@adamantike We appreciate your contribution and I do not want to reward your contribution by asking you to do extra work, but if you can find a way to enable the variable in a way that does not produce "code smell" we would prefer to maintain that kind of backward compatibility. I have not reviewed this change or #85 enough to have fully thought through the second order effects, so maybe we don't need to restore the variable. That is why I am asking the reviewers to consider it.

Also, to the extent you are interested in the changes in #77 and #90, it would be nice to have them all combined in one PR with a consistent coding style and coherent variable structure, so feel free to enhance this PR to include those features.

[korenyoni]

As another note, I'm noticing a lot of unused variables from prior to #85 in https://github.com/cloudposse/terraform-aws-s3-bucket/blob/master/examples/complete/variables.tf

[korenyoni]

@korenyoni Note to use as a reviewer: I think I might let it slide in this particular instance, but as a rule, when we find out a variable is no longer being used due to a recent change, I would prefer to re-enable the variable rather than remove it. We want to preserve backward compatibility as much as is reasonable.

@Nuru I agree, perhaps that's what we should have done in #85 and maybe deprecated the variables first before removing them outright. I think that at this point though, we're several versions past 0.35.0 (the automatically-drafted release following #85 ) so there's no point in attempting to restore this functionality. But I understand that this is more of a general note than a note for this particular case.

[korenyoni]

@adamantike sorry to make you do more work, but if we could take this opportunity to also remove unused variables following #85 in examples/complete

terraform-aws-s3-bucket/examples/complete/variables.tf

Lines 155 to 183 in d73a84e

	variable "standard_transition_days" {
	type = number
	default = 30
	description = "Number of days to persist in the standard storage tier before moving to the infrequent access tier"
	}
	variable "glacier_transition_days" {
	type = number
	default = 60
	description = "Number of days after which to move the data to the glacier storage tier"
	}
	variable "enable_glacier_transition" {
	type = bool
	default = true
	description = "Enables the transition to AWS Glacier which can cause unnecessary costs for huge amount of small files"
	}
	variable "enable_standard_ia_transition" {
	type = bool
	default = false
	description = "Enables the transition to STANDARD_IA"
	}
	variable "expiration_days" {
	type = number
	default = 90
	description = "Number of days after which to expunge the objects"
	}

and possibly any other variable that was missed, that would be best

[mergify]

This pull request is now in conflict. Could you fix it @adamantike? 🙏

[korenyoni]

At this point in time we are on 0.45.0, which is even further away from 0.35.0 when this variable stopped being used. I'm just going to go ahead and proceed with this change without any attempt at backwards compatibility with anything prior to 0.35.0.

@Nuru I agree with you in principle, but in this particular case it's a lost cause (a difference of 10 minor versions).

[aknysh]

tis is already a part of variable "lifecycle_rules", we can delete it

This Pull Request has been updated, so we're dismissing all reviews.

[korenyoni]

/test all