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

Removed unicode_str & fun_stacktrace #171

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Conversation

ariel-anieli
Copy link

  • CI/CD uses R23 and onwards
  • unicode_str (introduced in f8f72b7) to work around R20 compile warnings
  • fun_stacktrace (introduced in ad2d57d) relies on erlang:get_stacktrace/0, deprecated from R23.

src/ec_date.erl Outdated
@@ -709,11 +709,7 @@ pad6(X) when is_integer(X) ->
ltoi(X) ->
list_to_integer(X).

-ifdef(unicode_str).
uppercase(Str) -> string:uppercase(Str).
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we just keep the intermediary call here or also fold in the uppercase/1 call to be string:uppercase/1?

Copy link
Author

Choose a reason for hiding this comment

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

Folded all down to string:uppercase/1: no blocker.

@@ -99,16 +99,10 @@ parse_tags(Pattern) ->
{Tag, Vsn1}
end.

-ifdef(unicode_str).
len(Str) -> string:length(Str).
trim(Str, right, Chars) -> string:trim(Str, trailing, Chars);
trim(Str, left, Chars) -> string:trim(Str, leading, Chars).
Copy link
Collaborator

Choose a reason for hiding this comment

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

same question here. Can we or should we remove these intermediary calls? Any blockers?

Copy link
Author

Choose a reason for hiding this comment

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

No blocker.

src/ec_talk.erl Outdated
@@ -197,13 +197,8 @@ get_string(String) ->
no_clue
end.

-ifdef(unicode_str).
trim(Str) -> string:trim(Str).
trim(Str, both, Chars) -> string:trim(Str, both, Chars).
Copy link
Collaborator

Choose a reason for hiding this comment

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

And same question here.

Copy link
Author

Choose a reason for hiding this comment

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

No blocker.

@@ -886,7 +871,6 @@ runmany_wrap(Fun, Parent) ->
throw:R:Stacktrace ->
Parent ! {erlang:self(), error, {{nocatch, R}, Stacktrace}}
end.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one definitely needs to stay around.

Copy link
Author

Choose a reason for hiding this comment

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

Sure.

@ariel-anieli
Copy link
Author

Rebasing the PR; one extra commit has gone thru.

Ariel Otilibili added 2 commits March 7, 2024 06:58
* introduced in ad2d57d
* CI/CD uses R23 and onwards
* erlang:get_stacktrace/0 removed in R23.

[1] https://www.erlang.org/doc/general_info/removed#functions-removed-in-otp-23
* introduced in f8f72b7
* introduced for working around compile warning starting from R20
* CI/CD uses R23 and onwards.
@ariel-anieli
Copy link
Author

Good; please go ahead.

@ariel-anieli ariel-anieli requested a review from ferd March 7, 2024 06:05
@ferd ferd merged commit cb39837 into erlware:master Mar 13, 2024
3 checks passed
@ariel-anieli ariel-anieli deleted the pr-stacktrace branch March 18, 2024 00:06
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.

2 participants