8000 deprecate FixedArray::get by myfreess · Pull Request #2149 · moonbitlang/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

deprecate FixedArray::get #2149

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

myfreess
Copy link
Contributor
@myfreess myfreess commented May 22, 2025

Next step: #2146

Copy link
Deprecation notice should include migration example

Category
Maintainability
Code Snippet
#deprecated("use arr[i] instead")
pub fn FixedArray::get[T](self : FixedArray[T], idx : Int) -> T
Recommendation
Add more detailed deprecation notice with example:
#deprecated("use array indexing syntax 'arr[i]' instead. Example: replace 'arr.get(0)' with 'arr[0]'")
Reasoning
When deprecating APIs, it's important to provide clear migration paths with concrete examples to help users update their code. The current message is concise but could be more helpful with an explicit example.

Missing documentation about bounds checking behavior difference

Category
Maintainability
Code Snippet
pub fn FixedArray::get[T](self : FixedArray[T], idx : Int) -> T = "%fixedarray.get"
Recommendation
Update deprecation notice to clarify any differences in bounds checking behavior between get() and [] syntax
Reasoning
If there are any differences in runtime behavior (particularly around bounds checking) between the old get() method and the new [] syntax, this should be documented to prevent subtle bugs during migration

Consider adding compile-time warning for deprecated method

Category
Maintainability
Code Snippet
#deprecated
impl FixedArray {
get[T](Self[T], Int) -> T
Recommendation
Ensure the compiler generates appropriate warnings when this deprecated method is used and consider adding a target version for removal
Reasoning
Proper deprecation warnings help users identify and update deprecated API usage during development rather than discovering issues in production. A target version for removal helps with planning updates.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 6860

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 91.231%

Totals Coverage Status
Change from base Build 6851: 0.0%
Covered Lines: 6336
Relevant Lines: 6945

💛 - Coveralls

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
0