search

martinsson / Refactoring-Kata-Lift-Pass-Pricing

A refactoring kata where you'll practice disentangling business logic from frameworks and SQL
97 stars 81 forks source link

Setup CI for checking sanity of with_tests branch #21

Closed martinsson closed 2 years ago

martinsson commented 5 years ago

It'd be great to have a CI (travis, circleci, ..) to avoid systematically testing all PRs to the with_tests branch by hand.

Note: Possibly one could imagine to do the same for the master branch, but that'd require the tests to actually pass whereas now they fail in order for people to see where to start. Either way let's start with configuring a CI on the with_tests branch

codecop commented 5 years ago

Test branch could also always merge against master. there are conflicts we have to do something

rolger commented 2 years ago

Hi Johan

is this still an open issue? I'd like to implement a github-action for this issue. I'd start with Java

codecop commented 2 years ago

Yes please do. I can review your changes then

rolger commented 2 years ago

Alright mission completed :-) I need to prepare a PR. The changes are not just about adding the workflow action, need to fix the mysql driver and the unit-test startup.

You'll find the unit-test startup fix in this PR: https://github.com/martinsson/Refactoring-Kata-Lift-Pass-Pricing/pull/47

I'd suggest to merge this PR first and then I'll create a new one based on this. Or do you have another suggestion?

rolger commented 2 years ago

By the way a csharp implementation of the workflow should be easy as soon as the Java actions are available. So I'd continue to implement the same for csharp.

martinsson commented 2 years ago

Great job. Indeed it's still an open issue

On Fri, Feb 25, 2022, 9:39 AM Roland Germ @.***> wrote:

By the way a csharp implementation of the workflow should be easy as soon as the Java actions are available. So I'd continue to implement the same for csharp.

— Reply to this email directly, view it on GitHub https://github.com/martinsson/Refactoring-Kata-Lift-Pass-Pricing/issues/21#issuecomment-1050648079, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABMPQHX64GCVD7SREZZWBTU4455HANCNFSM4HDFBKCQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.*** .com>

rolger commented 2 years ago

Just completed the csharp pipeline. So how should I split up the PRs? What do we want to merge into the master branch?

rolger commented 2 years ago

Just completed the csharp pipeline. So how should I split up the PRs? What do we want to merge into the master branch?

Why I'm asking? First, the changes can be merged into the master branch easily. The code will not be executed on master, because I added a filter. But the master and with_test branch are out of sync. So I believe that merging from master to with_test won't be easy at all. Second, I need two additional changes, which don't relate to the feature but must be fixed upfront:

  1. The JUnit test should start the spark server only once
  2. An upgrade of the mysql-connector-java version to 8.0.28 (see https://stackoverflow.com/questions/53131321/spring-boot-jdbc-javax-net-ssl-sslexception-closing-inbound-before-receiving-p/53136942#53136942)
codecop commented 2 years ago

Merge to master is good idea. It should be possible from with-tests. I recommend one large pr for with-tests branch. Merge is a separate task we can do afterwards. I believe there are also things missing on with-tests

martinsson commented 2 years ago

Hi how could I make this easier? By merging master into with_tests to make it up to date?

codecop commented 2 years ago

@martinsson Johan maybe first drop the PR's which want to bump versions - unless you want that. I usually deactivate Security scans in kata projects. Second please accept or reject other open pull requests. I commented on PRs for that.

Also there is a simplify C# branch, can it be brougt to master in separate folder like java-simple? That would be great.

codecop commented 2 years ago

@rolger could you also add actions for java-simple and csharp-simple when available. In fact everything should be tested, also Scala and PHP and such. I have many github integrations in https://github.com/codecop/Parrot-Refactoring-Kata/tree/github-build

rolger commented 2 years ago

@codecop I'll have a look at them.

By the way CSharp needs an upgrade. The .Net version is not supported anymore.

martinsson commented 2 years ago

@martinsson Johan maybe first drop the PR's which want to bump versions - unless you want that. I usually deactivate Security scans in kata projects. Second please accept or reject other open pull requests. I commented on PRs for that.

Good point

Also there is a simplify C# branch, can it be brougt to master in separate folder like java-simple? That would be great.

I'll have a look at that

rolger commented 2 years ago

@codecop I'll have a look at them.

By the way CSharp needs an upgrade. The .Net version is not supported anymore.

@martinsson: maybe you open a new issue for that. I can fix it.

rolger commented 2 years ago

I believe the issue can be closed. There are build scripts for all languages for the with_tests branch.

The scripts could be added and commited to the master branch, because they are matching only on the with_tests branch.

martinsson commented 2 years ago

Brilliant @rolger :)

martinsson commented 2 years ago

Done by #52