Skip to content

Conversation

bobzhang
Copy link
Contributor

@bobzhang bobzhang commented Oct 2, 2025

No description provided.

@bobzhang bobzhang requested a review from Copilot October 2, 2025 02:53
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes the start_offset method public for the ArrayView type, allowing external access to the starting offset of an array view.

  • Adds a public start_offset method that wraps the existing private start method
  • Exposes the starting offset functionality to external consumers of the ArrayView API

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
array/view.mbt Adds private start method and public start_offset wrapper method
array/pkg.generated.mbti Updates package interface to include the new public method signature

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

///|
fn[T] ArrayView::start(self : ArrayView[T]) -> Int = "%arrayview.start"
///|
Copy link
Preview

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

The documentation comments are empty (only contain ///|). These methods should have proper documentation describing their purpose, parameters, and return values.

Suggested change
///|
///
/// Returns the starting offset of the array view.
///
/// # Parameters
/// * `self`: The array view whose starting offset is to be returned.
///
/// # Returns
/// An integer representing the starting offset of the array view.
///
/// # Example
/// ```moonbit
/// let arr = [10, 20, 30, 40, 50]
/// let view = arr[2:4]
/// inspect(view.start_offset(), content="2")
/// ```

Copilot uses AI. Check for mistakes.

fn[T] ArrayView::start(self : ArrayView[T]) -> Int = "%arrayview.start"
///|
pub fn[T] ArrayView::start_offset(self : Self[T]) -> Int {
Copy link
Preview

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

Inconsistent type parameter usage. The start method uses ArrayView[T] while start_offset uses Self[T]. For consistency, both should use the same pattern.

Suggested change
pub fn[T] ArrayView::start_offset(self : Self[T]) -> Int {
pub fn[T] ArrayView::start_offset(self : ArrayView[T]) -> Int {

Copilot uses AI. Check for mistakes.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 1449

Details

  • 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.009%) to 89.19%

Changes Missing Coverage Covered Lines Changed/Added Lines %
array/view.mbt 0 1 0.0%
Totals Coverage Status
Change from base Build 1448: -0.009%
Covered Lines: 9274
Relevant Lines: 10398

💛 - Coveralls

@bobzhang bobzhang merged commit 746b8d8 into main Oct 2, 2025
14 checks passed
@bobzhang bobzhang deleted the hongbo/start_offset_pub branch October 2, 2025 20:55
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