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

Link to log diff between repost in "To upstream" badge #264

Merged
merged 5 commits into from
Nov 8, 2024

Conversation

krzema12
Copy link
Owner

@krzema12 krzema12 commented Nov 8, 2024

Part of #260.

Thanks to this, we can preview what commits need to be synced.

@krzema12 krzema12 requested review from aSemy and OptimumCode November 8, 2024 07:45
@krzema12 krzema12 changed the title Link to log diff between repos Link to log diff between repost in "To upstream" badge Nov 8, 2024
@krzema12 krzema12 enabled auto-merge (squash) November 8, 2024 08:42
Copy link
Owner Author

@krzema12 krzema12 left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'SnakeKMP benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 1a4aa28 Previous: abe514a Ratio
macosX64.LoadingTimeBenchmark.loadsOpenAiSchema ( {"openAiYamlPath":"data/issues/kmp-issue-204-OpenAI-API.yaml"} ) 215.2954947392857 ms/op 121.74859682558142 ms/op 1.77

This comment was automatically generated by workflow using github-action-benchmark.

@OptimumCode
Copy link
Collaborator

⚠️ Performance Alert ⚠️

What do you think about excluding macosX64 from benchmarks? It is a bit flaky in the last couple of months (in my repo as well)

OptimumCode
OptimumCode previously approved these changes Nov 8, 2024
@krzema12
Copy link
Owner Author

krzema12 commented Nov 8, 2024

⚠️ Performance Alert ⚠️

What do you think about excluding macosX64 from benchmarks? It is a bit flaky in the last couple of months (in my repo as well)

I'm fine with this, we have enough coverage for other platforms and these benchmarks should catch regressions. Do you have a moment to take care of it? 🙏

@krzema12 krzema12 requested a review from OptimumCode November 8, 2024 10:57
UPSTREAM_COMMIT=$(cat ../upstream-commit.txt)
echo "See in BitBucket: https://bitbucket.org/snakeyaml/snakeyaml-engine/branches/compare/master..${'$'}UPSTREAM_COMMIT" > ../$logDiffBetweenRepos
echo "" >> ../$logDiffBetweenRepos
git log --oneline ${'$'}UPSTREAM_COMMIT..master >> ../$logDiffBetweenRepos
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: maybe introduce a shell variable with the command to avoid duplication?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I tried, but the new lines disappear when writing git's output stored in a variable to a text file. For sure there's a way to make it work, but I just think we can go with this minor duplication, unless you know from the top of your head how to fight this Bash peculiarity? 😁

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, I don't unfortunately)

But I meant that you could extract the command itself into a variable and execute it like sh -c $command or eval $command (for bash)

@krzema12 krzema12 merged commit 373d897 into main Nov 8, 2024
14 checks passed
@krzema12 krzema12 deleted the link-to-log-diff-between-repos branch November 8, 2024 11:28
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