Skip to content

fix: Replace deprecated String.prototype.substr()#1242

Merged
phil-davis merged 1 commit into
owncloud:masterfrom
CommanderRoot:refactor/rm-deprecated-substr
Jul 10, 2026
Merged

fix: Replace deprecated String.prototype.substr()#1242
phil-davis merged 1 commit into
owncloud:masterfrom
CommanderRoot:refactor/rm-deprecated-substr

Conversation

@CommanderRoot

Copy link
Copy Markdown
Contributor

String.prototype.substr() is deprecated so we replace it with String.prototype.slice() which works similarily but isn't deprecated.
.substr() probably isn't going away anytime soon but the change is trivial so it doesn't hurt to do it.

@sonarqubecloud

sonarqubecloud Bot commented Apr 4, 2022

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@mmattel

mmattel commented Aug 26, 2022

Copy link
Copy Markdown

@phil-davis Technically this looks feasable...

@dj4oC dj4oC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Security: no findings — mechanical .substr().slice() replacement, no behavior change intended
  • Stability: GitHub reports this PR as mergeable_state: blocked (likely merge conflicts or a required check gate) and it's had no activity since Aug 2022; 0 check runs on file for the head commit, so CI has never actually validated this against current master.
  • Performance: no findings
  • Test coverage: no tests added, but this is a pure API-rename refactor across 13 files with no logic change — low risk either way
  • TODOs found: 0
  • Dependency touched: no
  • CI status: none reported (stale, blocked)

Verdict

Idea is fine and low-risk, but the PR is 4+ years stale and blocked/unmergeable in its current state. Needs a rebase against current master and a fresh CI run before it can be merged — not actionable as-is.

🤖 Automated review by Claude Code (security · stability · performance · coverage)


Generated by Claude Code

@CommanderRoot CommanderRoot force-pushed the refactor/rm-deprecated-substr branch from 45eebbe to ed736f3 Compare July 9, 2026 17:01
@CommanderRoot CommanderRoot requested a review from a team as a code owner July 9, 2026 17:01
@CommanderRoot

Copy link
Copy Markdown
Contributor Author

Rebased against current master as requested by our overlords

.substr() is deprecated so we replace it with functions which work similarily but aren't deprecated

Signed-off-by: Tobias Speicher <[email protected]>
@phil-davis phil-davis force-pushed the refactor/rm-deprecated-substr branch from ed736f3 to c304a2e Compare July 10, 2026 05:00
@phil-davis phil-davis changed the title Replace deprecated String.prototype.substr() fix: Replace deprecated String.prototype.substr() Jul 10, 2026
@phil-davis

phil-davis commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

I adjusted the commit and PR title to meet the new semantic commit rules.

@phil-davis phil-davis merged commit 90cce04 into owncloud:master Jul 10, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants