Skip to content

Conversation

LiYing2010
Copy link
Contributor

Change List

  • fixed code style of sample code
  • fixed language of sample code
  • fixed some incorrect content

@LiYing2010 LiYing2010 requested a review from a team as a code owner August 5, 2025 10:13
@@ -100,7 +100,7 @@ You can enable all the supported ES2015 features at once by adding the `es2015`

```kotlin
tasks.withType<KotlinJsCompile>().configureEach {
kotlinOptions {
compilerOptions {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

JFYI
IMO:
kotlinOptions is deprecated, should use compilerOptions instead

Comment on lines 307 to 312
If you wanted to create a new class which implements the `Drawable` interface, and have the same behavior with `Circle`
class **except** for the value of the `color` property, you still need to add implementations for each member function
of the `Drawable` interface:

```kotlin
class RedCircle(val circle: Circle) : Circle {

class RedCircle(val circle: Circle) : Drawable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

JFYI
IMO:
if class RedCircle extends class Circle, it will inherit all the member functions, which means there will be no necessary to implement func draw() and resize()

so I think the sample code here will confuse the readers, and I suggest to change it

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right that this example could be improved. I'm covering this in #5026 Can you revert the changes in this file since I'll replace it very soon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK
I reverted this change

@@ -37,7 +37,7 @@ Create a project using the Kotlin Multiplatform wizard:
>
{style="note"}

2. In **composeApp** | **Tasks** | **kotlin browser**, select and run the **wasmJsBrowserDevelopmentRun** task.
2. In **wasmdemo** | **Tasks** | **kotlin browser**, select and run the **wasmJsBrowserDevelopmentRun** task.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

JFYI
the demo project name was changed to WasmDemo, but this content still use the old name composeApp

@@ -235,7 +235,7 @@ This feature tells the compiler to apply the annotation to all relevant parts of

* **`get`**: the getter method.

* **`set_param`**: the parameter of the setter method, if the property is defined as `var`.
* **`setparam`**: the parameter of the setter method, if the property is defined as `var`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO
I think setparam is the correct use-site target name

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, it is! We fixed the KEEP as well. Kotlin/KEEP@568ecac

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I merged your changes from master branch

@sarahhaggarty sarahhaggarty self-assigned this Aug 5, 2025
@LiYing2010
Copy link
Contributor Author

@sarahhaggarty
could you please review this?
thank you~~~

@LiYing2010 LiYing2010 requested a review from daniCsorbaJB August 5, 2025 16:36
Copy link
Collaborator

@sarahhaggarty sarahhaggarty left a comment

Choose a reason for hiding this comment

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

Thanks for your suggestions @LiYing2010 !
Can you rebase your branch onto the latest on master to pick up the changes we made in annotations.md? And please take a look at my suggestions for changes. Let me know if you have any questions!

@@ -63,7 +63,7 @@ In the following table, there are the minimum and maximum **fully supported** ve
| 1.7.0–1.7.10 | 6.7.1–7.0.2 | 3.4.3–7.0.2 |
| 1.6.20–1.6.21 | 6.1.1–7.0.2 | 3.4.3–7.0.2 |

> *Kotlin 2.0.20–2.0.21 and Kotlin 2.1.0–2.1.10 are fully compatible with Gradle up to 8.6.
> Kotlin 2.0.20–2.0.21 and Kotlin 2.1.0–2.1.10 are fully compatible with Gradle up to 8.6.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
> Kotlin 2.0.20–2.0.21 and Kotlin 2.1.0–2.1.10 are fully compatible with Gradle up to 8.6.
> *Kotlin 2.0.20–2.0.21 and Kotlin 2.1.0–2.1.10 are fully compatible with Gradle up to 8.6.

This asterisk refers to an entry in the table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK
I reverted this change

@@ -235,7 +235,7 @@ This feature tells the compiler to apply the annotation to all relevant parts of

* **`get`**: the getter method.

* **`set_param`**: the parameter of the setter method, if the property is defined as `var`.
* **`setparam`**: the parameter of the setter method, if the property is defined as `var`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, it is! We fixed the KEEP as well. Kotlin/KEEP@568ecac

Comment on lines 250 to 251
// Applies @Email to `param`, `property`, `field`,
// `get`, and `setparam` (if var)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Applies @Email to `param`, `property`, `field`,
// `get`, and `setparam` (if var)
// Applies @Email to param, property, field,
// get, and setparam (if var)

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use backticks in code comments. I fixed this in the original doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I merged your changes from master branch

Comment on lines 254 to 255
// Applies @Email to `property`, `field`, and `get`
// (no `param` since it's not in the constructor)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Applies @Email to `property`, `field`, and `get`
// (no `param` since it's not in the constructor)
// Applies @Email to property, field, and get
// (no param since it's not in the constructor)

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use backticks in code comments. I fixed this in the original doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I merged your changes from master branch

Comment on lines 307 to 312
If you wanted to create a new class which implements the `Drawable` interface, and have the same behavior with `Circle`
class **except** for the value of the `color` property, you still need to add implementations for each member function
of the `Drawable` interface:

```kotlin
class RedCircle(val circle: Circle) : Circle {

class RedCircle(val circle: Circle) : Drawable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right that this example could be improved. I'm covering this in #5026 Can you revert the changes in this file since I'll replace it very soon?

# Conflicts:
#	docs/topics/annotations.md
#	docs/topics/whatsnew/whatsnew22.md
@sarahhaggarty sarahhaggarty merged commit 024c38a into JetBrains:master Aug 26, 2025
4 checks passed
@sarahhaggarty
Copy link
Collaborator

Thanks again for your contribution @LiYing2010 ! 🚀

@LiYing2010 LiYing2010 deleted the minor_fix branch August 26, 2025 09:03
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