Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove obsolete input and pass-through of audience #21

Closed
wants to merge 1 commit into from

Conversation

lusitania
Copy link
Contributor

Description

Removes ineffective input tfc_aws_audience and replaces aws_iam_openid_connect_provider.this.client_id_list with the only value used globally: aws.workload.identity.

Related boilerplate resources random_string.sfx1 and random_string.sfx2 removed as well.

How Has This Been Tested?

  1. terraform validate
  2. Search over both orgs
    • org:urbanmedia audience
    • org:tagesspiegel audience

No functional use. Only IaC-terraform-cloud passes the output through which needs removal. This constitutes a breaking change.

Test Configuration:

  • Terraform version: 1.9.2
  • Module version:

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@lusitania lusitania added the breaking-change Denotes a PR that changes CODE that affects the "API" label Jul 24, 2024
@lusitania lusitania requested a review from a team as a code owner July 24, 2024 15:30
@github-actions github-actions bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. fix Denotes a PR that fixes a problem or bug. labels Jul 24, 2024
@lusitania lusitania force-pushed the fix/remove_obsolete_audience branch from 31da54a to da55d68 Compare July 24, 2024 15:39
@lusitania lusitania self-assigned this Jul 24, 2024
@leonsteinhaeuser leonsteinhaeuser added the refactor Denotes a PR that refactors a specific part of an application. label Jul 24, 2024
@lusitania lusitania removed the fix Denotes a PR that fixes a problem or bug. label Jul 25, 2024
Copy link
Contributor

@leonsteinhaeuser leonsteinhaeuser left a comment

Choose a reason for hiding this comment

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

Leave it configurable. If something changes in the future, this module would have to be patched. Instead, people could just change it themselves without waiting for us to patch this module.

@lusitania
Copy link
Contributor Author

Leave it configurable. If something changes in the future, this module would have to be patched. Instead, people could just change it themselves without waiting for us to patch this module.

I strongly disagree. This is a prime example of boilerplate. "People" is "us". I am aware this module was made public, however the only commitment we have is to Tagesspiegel. If someone else finds this code useful they are not only free but encouraged to fork the project.

@leonsteinhaeuser
Copy link
Contributor

Leave it configurable. If something changes in the future, this module would have to be patched. Instead, people could just change it themselves without waiting for us to patch this module.

I strongly disagree. This is a prime example of boilerplate. "People" is "us". I am aware this module was made public, however the only commitment we have is to Tagesspiegel. If someone else finds this code useful they are not only free but encouraged to fork the project.

This is not the way it works. This module is automatically pushed to the public Terraform registry and you are supposed to use it from there, not from GitHub. Forking is only intended if you want to introduce a change/fix. Why make it more complicated when it works and gives us and others more flexibility?

@lusitania
Copy link
Contributor Author

Leave it configurable. If something changes in the future, this module would have to be patched. Instead, people could just change it themselves without waiting for us to patch this module.

I strongly disagree. This is a prime example of boilerplate. "People" is "us". I am aware this module was made public, however the only commitment we have is to Tagesspiegel. If someone else finds this code useful they are not only free but encouraged to fork the project.

This is not the way it works. This module is automatically pushed to the public Terraform registry and you are supposed to use it from there, not from GitHub. Forking is only intended if you want to introduce a change/fix. Why make it more complicated when it works and gives us and others more flexibility?

This is the way it works. Code is meant to serve a purpose. Boilerplate does not. Please check your lecture noted on Software Engineering. Can send you mine.

@lusitania
Copy link
Contributor Author

I'll resolve this differently. This change is now obsolete.

@lusitania lusitania closed this Jul 29, 2024
@lusitania lusitania deleted the fix/remove_obsolete_audience branch July 29, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Denotes a PR that changes CODE that affects the "API" refactor Denotes a PR that refactors a specific part of an application. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants