Skip to content

Conversation

@TomaszGaweda
Copy link
Contributor

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

How do you envisage this behind useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each code sample should have a test.

If any tests becomes flaky or simply broken, we should report a test failure just as we do for main hazelcast repo

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this might be over-engineered.

AFAIK we don't want test failures in GitHub for internal stuff, but do we really expect community users to address them? And do we expect the tests to be complicated enough to become flakey?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we really expect community users to address them

That's a very good "first time issue" stuff :)

do we expect the tests to be complicated enough to become flakey

Well I've encountered some tests that were flaky on MacOS (preferIPV4 FTW)

Fixes INSERT_LINK_TO_THE_ISSUE_HERE

Checklist:
- [ ] Request reviewers if possible
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for internal or external users? Isn’t this implicit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both, but mostly external

Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering how an external user would identify reviewers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Git blame who wrote the test ;) GitHub already does that when you modify some files, it tells you who edited them before

@JackPGreen
Copy link
Contributor

I'm not a fan of these changes as it seems over-engineered. But then I think actually these are just copied from hazelcast? In that case it's probably fine?


Fixes INSERT_LINK_TO_THE_ISSUE_HERE

Checklist:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering whether this should include documentation updates. If something's useful enough to prompt a new code sample, should it be in the docs too? What's the process for making that happen?

What do you think @oliverhowell ?

@TomaszGaweda
Copy link
Contributor Author

Some points are copied, some are new/adjusted to this repo. Idea is from core hazelcast indeed to guide people how to contribute by PRs and issues.

@TomaszGaweda TomaszGaweda merged commit b1a2e76 into hazelcast:master Feb 17, 2025
3 checks passed
@TomaszGaweda TomaszGaweda deleted the 6.0/pr-template branch February 17, 2025 14:47
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.

4 participants