Skip to content

Conversation

@peter-jerry-ye
Copy link
Collaborator

Closes #1787

The reason for deprecation is:

  1. it can be replaced with matrix functions
  2. the order of parameters is not nature

We may or may not add back the version where the parameters are swapped.

@peter-jerry-ye-code-review
Copy link

peter-jerry-ye-code-review bot commented Apr 27, 2025

Deprecated function alternatives could be better documented

Category
Maintainability
Code Snippet
#deprecated("use tuple |> then(fn { (a, b) => (f(a), b) }) instead")
Recommendation
Add more context in deprecation messages, including why the change is recommended and benefits of the new approach. Example:
#deprecated("use tuple |> then(fn { (a, b) => (f(a), b) }) instead for better composability and more intuitive parameter ordering")
Reasoning
Clear deprecation messages help users understand why they should migrate and how to best use the new approach

README examples could show more real-world use cases

Category
Maintainability
Code Snippet
let _tuple2 = (fn { (x, y) => (x + 1, y) })(tuple) // tuple2 = (2, 2)
Recommendation
Add examples that demonstrate practical use cases and show how the new approach can be chained:

let transformed = data
  |> then(fn { (x, y) => (x * 2, y) })
  |> then(fn { (x, y) => (x, y.to_string()) })```
**Reasoning**
Real-world examples help users better understand when and how to use the new matrix function approach

</details>
<details>

<summary> Inconsistent usage of the pipe operator in examples </summary>

**Category**
Correctness
**Code Snippet**
let _tuple2 = (fn { (x, y) => (x + 1, y) })(tuple) // tuple2 = (2, 2)
let _tuple3 = tuple |> then(fn { (x, y) => (x, y + 1) })
**Recommendation**
Consistently use the pipe operator throughout examples:
```moonbit
let _tuple2 = tuple |> then(fn { (x, y) => (x + 1, y) })```
**Reasoning**
Using consistent syntax in examples makes the code more predictable and easier to follow

</details>

@peter-jerry-ye peter-jerry-ye force-pushed the zihang/deprecate-tuple-functions branch from bef66c3 to 35cddae Compare April 27, 2025 09:44
@coveralls
Copy link
Collaborator

coveralls commented Apr 27, 2025

Pull Request Test Coverage Report for Build 6469

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 92.7%

Totals Coverage Status
Change from base Build 6467: 0.01%
Covered Lines: 6121
Relevant Lines: 6603

💛 - Coveralls

@bobzhang bobzhang merged commit e518158 into main Apr 28, 2025
12 checks passed
@bobzhang bobzhang deleted the zihang/deprecate-tuple-functions branch April 28, 2025 00:57
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.

@tuple.map_fst @tuple.map_snd @tuple.map_both can't use pipeline operator and dot syntax

4 participants