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

deprecate: inconsistent api @sorted_map.of #1593

Merged
merged 2 commits into from
Feb 7, 2025

Conversation

illusory0x0
Copy link
Contributor

I think deprecate old API is impossible

@coveralls
Copy link
Collaborator

coveralls commented Feb 5, 2025

Pull Request Test Coverage Report for Build 5192

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 85.798%

Totals Coverage Status
Change from base Build 5188: 0.0%
Covered Lines: 5437
Relevant Lines: 6337

💛 - Coveralls

Copy link
Collaborator

@peter-jerry-ye peter-jerry-ye left a comment

Choose a reason for hiding this comment

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

The from_array should also be implemented

pub fn of[K : Compare, V](entries : Array[(K, V)]) -> T[K, V] {
///|
/// Creates a sorted map from a fixed array of key-value pairs.
pub fn of[K : Compare, V](entries : FixedArray[(K, V)]) -> T[K, V] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a breaking change: not a deprecation.

For deprecation, you should use deprecation warning, tell people what to do, and then change the API in the next release

Suggested change
pub fn of[K : Compare, V](entries : FixedArray[(K, V)]) -> T[K, V] {
/// @alert deprecated "Use @sorted_map.from_array instead"
pub fn of[K : Compare, V](entries : Array[(K, V)]) -> T[K, V] {

@illusory0x0 illusory0x0 changed the title fix: inconsistent api of deprecate: inconsistent api @sorted_map.of Feb 7, 2025
@illusory0x0
Copy link
Contributor Author

bleeding-check failed is another issus.

 91 │ pub fn is[T : Show](a : T, b : T, loc~ : SourceLoc = _) -> Unit! {
    │        ─┬  
    │         ╰── Error (warning): The word `is` is reserved for possible future use. Please consider using another name.

@peter-jerry-ye
Copy link
Collaborator

bleeding-check failed is another issus.

 91 │ pub fn is[T : Show](a : T, b : T, loc~ : SourceLoc = _) -> Unit! {
    │        ─┬  
    │         ╰── Error (warning): The word `is` is reserved for possible future use. Please consider using another name.

Don't worry, it's just a grammar change.

@peter-jerry-ye peter-jerry-ye merged commit f823620 into moonbitlang:main Feb 7, 2025
13 of 16 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.

3 participants