8000 Group deprecated blocks by bobzhang · Pull Request #2316 · moonbitlang/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Group deprecated blocks #2316

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 23, 2025
Merged

Group deprecated blocks #2316

merged 1 commit into from
Jun 23, 2025

Conversation

bobzhang
Copy link
Contributor

Summary

  • move deprecated deque functions into deprecated.mbt
  • move deprecated list functions into deprecated.mbt
  • move deprecated double functions into deprecated.mbt

Testing

  • moon fmt
  • moon info
  • moon test
  • moon check

https://chatgpt.com/codex/tasks/task_e_6858faf57c248320aebb9138fa34cf6e

@peter-jerry-ye-code-review peter-jerry-ye code review
Copy link
Comments should be moved together with deprecated functions

Category
Maintainability
Code Snippet
pub fn[A] pop_front_exn(self : T[A]) -> Unit {
self.unsafe_pop_front()
}
Recommendation
Move the original documentation comments along with deprecated functions to maintain API documentation history
Reasoning
Even for deprecated functions, maintaining documentation is important for legacy code understanding and migration guidance. The examples and detailed descriptions should be preserved in deprecated.mbt

Consider grouping deprecated functions by version/timeline

Category
Maintainability
Code Snippet
// double/deprecated.mbt
pub fn Double::nan() -> Double
pub fn Double::inf(sign : Int) -> Double
pub fn Double::min_normal() -> Double
Recommendation
Add version information in comments or group functions by when they were deprecated to help with migration planning:

// Deprecated in v1.0
pub fn Double::nan() ...

// Deprecated in v1.1 
pub fn Double::inf ...

Reasoning
Grouping deprecated functions by when they were deprecated helps users understand the deprecation timeline and plan migrations accordingly

Add migration guide comments for deprecated functions

Category
Maintainability
Code Snippet
#deprecated("Use unsafe_pop_front instead")
Recommendation
Add more detailed migration guidance including code examples:

/// Migration guide:
/// Replace: dq.pop_front_exn()
/// With: dq.unsafe_pop_front()
#deprecated("Use `unsafe_pop_front` instead")

Reasoning
Simply stating the replacement function may not be enough. Providing concrete migration examples makes it easier for users to update their code

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 7444

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 90.117%
Totals Coverage Status
Change from base Build 7440: 0.0%
Covered Lines: 8544
Relevant Lines: 9481
💛 - Coveralls

@bobzhang bobzhang enabled auto-merge (rebase) June 23, 2025 08:54
@bobzhang bobzhang merged commit 8c8da02 into main Jun 23, 2025
16 checks passed
@bobzhang bobzhang deleted the codex/group-deprecated-functions-into-deprecated.mbt branch June 23, 2025 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
codex
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bobzhang @coveralls
0