Skip to content

[TT-11739] Fix RateLimitExceeded var name to include Event prefix#6319

Merged
titpetric merged 1 commit into
masterfrom
fix/tt-11739/alias-rate-limiter-types-for-compat
May 30, 2024
Merged

[TT-11739] Fix RateLimitExceeded var name to include Event prefix#6319
titpetric merged 1 commit into
masterfrom
fix/tt-11739/alias-rate-limiter-types-for-compat

Conversation

@titpetric

@titpetric titpetric commented May 30, 2024

Copy link
Copy Markdown
Contributor

PR Type

Bug fix


Description

  • Renamed RateLimitExceeded to EventRateLimitExceeded in gateway/event_system.go to ensure naming consistency with other event aliases.
  • Maintained backward compatibility by updating the alias.

Changes walkthrough 📝

Relevant files
Bug fix
event_system.go
Rename RateLimitExceeded to EventRateLimitExceeded for consistency

gateway/event_system.go

  • Renamed RateLimitExceeded to EventRateLimitExceeded for consistency.
  • Maintained backward compatibility with alias changes.
  • +1/-1     

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

    @titpetric titpetric changed the title Fix var name [TT-11739] Fix RateLimitExceeded var name to include Event prefix May 30, 2024
    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Description updated to latest commit (f45a888)

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    API Changes

    --- prev.txt	2024-05-30 19:23:03.134959282 +0000
    +++ current.txt	2024-05-30 19:23:00.038943314 +0000
    @@ -7102,7 +7102,7 @@
     	// EventQuotaExceeded is an alias maintained for backwards compatibility.
     	EventQuotaExceeded = event.QuotaExceeded
     	// RateLimitExceeded is an alias maintained for backwards compatibility.
    -	RateLimitExceeded = event.RateLimitExceeded
    +	EventRateLimitExceeded = event.RateLimitExceeded
     	// EventAuthFailure is an alias maintained for backwards compatibility.
     	EventAuthFailure = event.AuthFailure
     	// EventKeyExpired is an alias maintained for backwards compatibility.

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    1, because the PR involves a simple renaming of a variable for consistency and maintains backward compatibility. The change is straightforward and limited to one file with a single line modification.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Possible Bug: The renaming of RateLimitExceeded to EventRateLimitExceeded might cause issues if external code references the old variable name. This could lead to runtime errors if not properly handled.

    🔒 Security concerns

    No

    Code feedback:
    relevant filegateway/event_system.go
    suggestion      

    Consider maintaining an alias for the old variable name RateLimitExceeded to ensure backward compatibility for any clients or services that might be using the old name. This can prevent potential runtime errors from unresolved references. [important]

    relevant lineEventRateLimitExceeded = event.RateLimitExceeded

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Rename the variable to clearly indicate it is an alias

    To maintain consistency and clarity, consider renaming EventRateLimitExceeded to
    EventRateLimitExceededAlias to explicitly indicate that this is an alias for backward
    compatibility.

    gateway/event_system.go [31]

    -EventRateLimitExceeded = event.RateLimitExceeded
    +EventRateLimitExceededAlias = event.RateLimitExceeded
     
    Suggestion importance[1-10]: 7

    Why: The suggestion improves code clarity by explicitly indicating that the variable is an alias for backward compatibility. However, it is a minor improvement and not crucial for functionality.

    7

    @titpetric
    titpetric enabled auto-merge (squash) May 30, 2024 19:23
    @sonarqubecloud

    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

    @titpetric
    titpetric merged commit 30cf2ef into master May 30, 2024
    @titpetric
    titpetric deleted the fix/tt-11739/alias-rate-limiter-types-for-compat branch May 30, 2024 19:42
    nerdydread pushed a commit that referenced this pull request Sep 6, 2024
    )
    
    ### **PR Type**
    Bug fix
    
    
    ___
    
    ### **Description**
    - Renamed `RateLimitExceeded` to `EventRateLimitExceeded` in
    `gateway/event_system.go` to ensure naming consistency with other event
    aliases.
    - Maintained backward compatibility by updating the alias.
    
    
    ___
    
    
    
    ### **Changes walkthrough** 📝
    <table><thead><tr><th></th><th align="left">Relevant
    files</th></tr></thead><tbody><tr><td><strong>Bug fix
    </strong></td><td><table>
    <tr>
      <td>
        <details>
    <summary><strong>event_system.go</strong><dd><code>Rename
    RateLimitExceeded to EventRateLimitExceeded for
    consistency</code></dd></summary>
    <hr>
    
    gateway/event_system.go
    <li>Renamed <code>RateLimitExceeded</code> to
    <code>EventRateLimitExceeded</code> for consistency.<br> <li> Maintained
    backward compatibility with alias changes.<br>
    
    
    </details>
        
    
      </td>
    <td><a
    href="https://github.com/TykTechnologies/tyk/pull/6319/files#diff-d56e22d4f1b8d2e91bb643d30e678a3819691a18bfae8506b10e0af8dc279a0e">+1/-1</a>&nbsp;
    &nbsp; &nbsp; </td>
    </tr>                    
    </table></td></tr></tr></tbody></table>
    
    ___
    
    > 💡 **PR-Agent usage**:
    >Comment `/help` on the PR to get a list of all available PR-Agent tools
    and their descriptions
    
    Co-authored-by: Tit Petric <[email protected]>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants