Skip to content
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

Make external functions isolated #91

Merged

Conversation

NipunaMadhushan
Copy link
Contributor

@NipunaMadhushan NipunaMadhushan commented Mar 20, 2024

Purpose

All the external functions should be isolated functions.

Fixes https://github.com/ballerina-platform/module-ballerinax-prometheus/issues/146

Examples

Checklist

  • Linked to an issue
  • Updated the changelog
  • Added tests

Copy link
Member

@MaryamZi MaryamZi left a comment

Choose a reason for hiding this comment

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

Hope we've verified that these are indeed safe - i.e., specifically for isolated functions, they meet the requirements for isolated functions implemented in Ballerina

ballerina/natives.bal Outdated Show resolved Hide resolved
ballerina/natives.bal Show resolved Hide resolved
ballerina/natives.bal Outdated Show resolved Hide resolved
ballerina/natives.bal Outdated Show resolved Hide resolved
ballerina/natives.bal Outdated Show resolved Hide resolved
ballerina/natives.bal Outdated Show resolved Hide resolved
@NipunaMadhushan
Copy link
Contributor Author

@MaryamZi I have re-added isolated qualifier for public functions. I bumped the minor version as well since this will be a breaking change.

ballerina/natives.bal Outdated Show resolved Hide resolved
ballerina/natives.bal Outdated Show resolved Hide resolved
} else {
self.metricTags = DEFAULT_TAGS;
lock {
self.metricTags = defaultTags.cloneReadOnly();
Copy link
Member

Choose a reason for hiding this comment

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

Formatting is off?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checked and seems like the formatting is correct. Anyway these locks have been removed

ballerina/natives.bal Outdated Show resolved Hide resolved
Copy link
Member

@MaryamZi MaryamZi left a comment

Choose a reason for hiding this comment

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

Added a minor comment. Other than that, code changes LGTM.

ballerina/natives.bal Outdated Show resolved Hide resolved
Copy link
Member

@MaryamZi MaryamZi left a comment

Choose a reason for hiding this comment

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

Code changes LGTM.

@NipunaMadhushan NipunaMadhushan merged commit 3e9a47e into ballerina-platform:main Aug 2, 2024
4 checks passed
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.

Concurrent calls warning when using this module
3 participants