Skip to content

[DX-1345] Update config description for inclusive naming project#6286

Merged
dcs3spp merged 2 commits into
masterfrom
dx-1322
May 20, 2024
Merged

[DX-1345] Update config description for inclusive naming project#6286
dcs3spp merged 2 commits into
masterfrom
dx-1322

Conversation

@dcs3spp

@dcs3spp dcs3spp commented May 16, 2024 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

User description

Description

Related Issue

Motivation and Context

Update config description for inclusive naming project

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

PR Type

Enhancement

Description

  • Updated the comments in config/config.go to use more inclusive language:
    • Replaced 'master' with 'primary' in the DisableKeySpaceSync comment.
    • Changed 'whitelist' to 'allow list' in the ProxySSLCipherSuites comment.

Changes walkthrough 📝

Relevant files
Enhancement
config.go
Update Comments for Inclusive Language                                     

config/config.go

  • Updated comments to use inclusive language by replacing 'master' with
    'primary' and 'whitelist' with 'allow list'.
    		•  +2/-2     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @dcs3spp
    dcs3spp requested review from andyo-tyk and titpetric May 16, 2024 11:27
    @github-actions

    github-actions Bot commented May 16, 2024

    Copy link
    Copy Markdown
    Contributor

    PR Description updated to latest commit (692bc00)

    @github-actions

    github-actions Bot commented May 16, 2024 •
    edited
    Loading

    Copy link
    Copy Markdown
    Contributor

    API Changes

    --- prev.txt	2024-05-20 13:38:04.751500642 +0000
+++ current.txt	2024-05-20 13:38:01.523496146 +0000
@@ -5228,7 +5228,7 @@
 	// Maximum TLS version for connection between Tyk and your upstream service.
 	ProxySSLMaxVersion uint16 `json:"proxy_ssl_max_version"`
 
-	// Whitelist ciphers for connection between Tyk and your upstream service.
+	// Allow list of ciphers for connection between Tyk and your upstream service.
 	ProxySSLCipherSuites []string `json:"proxy_ssl_ciphers"`
 
 	// This can specify a default timeout in seconds for upstream API requests.
@@ -5955,7 +5955,7 @@
 	// For an Self-Managed installation this can be left at `false` (the default setting). For Legacy Cloud Gateways it must be set to ‘true’.
 	BindToSlugsInsteadOfListenPaths bool `json:"bind_to_slugs"`
 
-	// Set this option to `true` if you don’t want to monitor changes in the keys from a master Gateway.
+	// Set this option to `true` if you don’t want to monitor changes in the keys from a primary Gateway.
 	DisableKeySpaceSync bool `json:"disable_keyspace_sync"`
 
 	// This is the `zone` that this instance inhabits, e.g. the cluster/data-centre the Gateway lives in.

    @github-actions

    github-actions Bot commented May 16, 2024

    Copy link
    Copy Markdown
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are straightforward and limited to renaming terms for inclusivity. The modifications are confined to comments and JSON tags, which are less likely to introduce functional regressions.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    No

    🔒 Security concerns

    No
    Code feedback:
    relevant fileconfig/config.go
    suggestion      

    Consider updating the JSON tag from disable_keyspace_sync to disable_keyspace_sync_primary to reflect the terminology change in the comment and maintain consistency. [medium]
    relevant line// Set this option to `true` if you don’t want to monitor changes in the keys from a primary Gateway.
    relevant fileconfig/config.go
    suggestion      

    Ensure that the renaming of terms does not affect any external documentation or API contracts that might still use the old terminology. If necessary, update those resources accordingly. [important]
    relevant line// Allow list of ciphers for connection between Tyk and your upstream service.

    @github-actions

    github-actions Bot commented May 16, 2024

    Copy link
    Copy Markdown
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Clarify the comment to enhance understanding and maintain consistency with inclusive terminology

    Update the comment for DisableKeySpaceSync to clarify what "primary Gateway" refers to,
    ensuring it aligns with the new inclusive terminology and enhances understanding for new
    developers or external contributors.

    config/config.go [339]

    -// Set this option to `true` if you don’t want to monitor changes in the keys from a primary Gateway.
+// Set this option to `true` if you don’t want to monitor changes in the keys from the main Gateway.
    Suggestion importance[1-10]: 7

    Why: The suggestion correctly identifies a change in terminology from "master" to "primary" in the PR and proposes further clarification. This enhances understanding and aligns with inclusive terminology.

    		7
    Clarity
    Use a more specific term to improve clarity and directness in the configuration

    Consider using a more specific term than "Allow list" for ProxySSLCipherSuites. The term
    "Allow list" is generic; specifying it as "AllowedCipherSuites" or similar could improve
    clarity and directness in the configuration.

    config/config.go [815]

    -// Allow list of ciphers for connection between Tyk and your upstream service.
+// List of allowed ciphers for connection between Tyk and your upstream service.
    Suggestion importance[1-10]: 6

    Why: The suggestion to refine the term "Allow list" to "AllowedCipherSuites" is valid as it improves clarity. However, the term "Allow list" is already a step towards more inclusive language, and the suggested change is a minor enhancement.

    		6
    Consistency
    Ensure consistent terminology across all configurations and documentation

    Ensure that the new terminology "Allow list" in ProxySSLCipherSuites is consistently
    applied across all configuration options and documentation to maintain uniformity and
    avoid confusion.

    config/config.go [815]

    -// Allow list of ciphers for connection between Tyk and your upstream service.
+// Approved list of ciphers for connection between Tyk and your upstream service.
    Suggestion importance[1-10]: 5

    Why: While the suggestion to ensure consistent terminology is generally good, it does not directly relate to a specific issue in the PR diff. It's more of a general best practice than a response to a specific change in the PR.

    		5

    @dcs3spp dcs3spp changed the title [DX-1322] Update config description for inclusive naming project [DX-1332] Update config description for inclusive naming project May 16, 2024
    @dcs3spp dcs3spp changed the title [DX-1332] Update config description for inclusive naming project [DX-1345] Update config description for inclusive naming project May 16, 2024
    @sonarqubecloud

    sonarqubecloud Bot commented May 20, 2024

    Copy link
    Copy Markdown

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    No data about Coverage
    0.0% Duplication on New Code

    See analysis details on SonarCloud

    @dcs3spp
    dcs3spp merged commit b816e91 into master May 20, 2024
    @dcs3spp
    dcs3spp deleted the dx-1322 branch May 20, 2024 15:16
    @dcs3spp

    dcs3spp commented May 20, 2024

    Copy link
    Copy Markdown
    Contributor Author

    /release to release-5.3

    @tykbot

    tykbot Bot commented May 20, 2024

    Copy link
    Copy Markdown

    Working on it! Note that it can take a few minutes.

    tykbot Bot pushed a commit that referenced this pull request May 20, 2024
    @dcs3spp
    [DX-1345] Update config description for inclusive naming project (#6286)
    5a1e4a3 
    Updated config.go for inclusive language tier 1 compliance

(cherry picked from commit b816e91)
    @buger buger mentioned this pull request May 20, 2024
    Merged
    @tykbot

    tykbot Bot commented May 20, 2024

    Copy link
    Copy Markdown

    @dcs3spp Succesfully merged PR

    buger added a commit that referenced this pull request May 20, 2024
    @buger
    Merging to release-5.3: [DX-1345] Update config description for inclu…
    05ec0f5 
    …sive naming project (#6286)

[DX-1345] Update config description for inclusive naming project (#6286)

Updated config.go for inclusive language tier 1 compliance
    nerdydread pushed a commit that referenced this pull request Sep 6, 2024
    @dcs3spp @nerdydread
    [DX-1345] Update config description for inclusive naming project (#6286)
    e9ca548 
    Updated config.go for inclusive language tier 1 compliance
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @andyo-tyk andyo-tyk andyo-tyk approved these changes
    @titpetric titpetric Awaiting requested review from titpetric

    Assignees

    No one assigned

    Labels

    enhancement Review effort [1-5]: 2

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @dcs3spp @andyo-tyk