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.

Sign up for GitHub

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

Jump to bottom

Integration test for laravel using docker and githubworkflow #119

Merged
merged 32 commits into from
May 12, 2022
Merged

Integration test for laravel using docker and githubworkflow #119

merged 32 commits into from
May 12, 2022

Conversation

Thiyagu55
Copy link
Collaborator

@Thiyagu55 Thiyagu55 commented May 2, 2022 •
edited
Loading

This sample applications are created using the command composer create-project laravel/laravel sqlcommenter-laravel

  1. The files added/modified by me are .github/workflows, app/http/controller/UserController and all files inside tests

Thiyagu55 and others added 30 commits March 18, 2022 15:54
@Thiyagu55
Initializing package
68efb74
@Thiyagu55
moving package composer.json to root
34d064f
@Thiyagu55
moving package composer.json to root
3abd1a4
@Thiyagu55
Google SQLcomenter
af703d6 
All Base functionalities
@Thiyagu55
Moving format comments to util
12e48a9
@Thiyagu55
Added unit tests for Util functions
5f87c4d
@Thiyagu55
Added Copyrights to individual packages
02c2f81
@Thiyagu55
PR code review changes
2b5d934
@Thiyagu55
URL Encoding for coments added
9f9d966
@Thiyagu55
Added additional config - action
5b88cb1
@Thiyagu55
Changing copyright year
9ef5fac
@Thiyagu55
Changing copyright year
2d99d7a
@Thiyagu55
Added sample for docker-laravel
e802658
@Thiyagu55
Test for Eloquent framework
b401291
@Thiyagu55
RawDB tests
8dfcda9
@Thiyagu55
Added support for ;
bde3747
@Thiyagu55
Removing print
7b3f3f0
@Thiyagu55
adding github docker workflow
57c3372
@Thiyagu55
Fixing error
13f57e9
@Thiyagu55
Fixing error
4b74557
@Thiyagu55
Updating php version in docker-laravel-test-runner to 8
0279e16
@Thiyagu55
updating environment variable in php-laravel docker github integratio…
420c299 
…n test
@Thiyagu55
Adding migration in github docker workflow
0025203
@Thiyagu55
Adding migration in github docker workflow
4fc2583
@Thiyagu55
Adding publishing SQLCommenter Configs in github integration test wor…
2fbb74c 
…kflow
@Thiyagu55
Adding unittest for laravel-integration-tests.yaml
ab80717
@Thiyagu55
Adding unittest for laravel-integration-tests.yaml
9e7d016
@Thiyagu55
Merge branch 'master' into docker-caravel
fd212ab
@Thiyagu55
Fixing merge commit error
1612bbd
@Thiyagu55
Laravel commenter sample code README.md
fdf3acd
@sjs994 sjs994 mentioned this pull request May 12, 2022
Open
@sjs994
Copy link
Collaborator

sjs994 commented May 12, 2022

I don't see any direct red flags in this CL. Contrary to that, we need it sooner than later to not have any breaking changes in the library. Added a ticket to review in more detail: #122.

@Thiyagu55, As a retrospective please follow the approach of not merging changes directly to local main branch and let it from from origin back to the main branch.
Also it would be better if we create multiple small PRs, like in this case we could have created 4 PRs sequentially:

  1. PR1: Add sample application without any change(mentioning the command used to create the sample PHP application)
  2. PR2: Add git flows for unit tests.
  3. PR3: Modify the changes in the sample application to use sqlcommenter-php library.
  4. PR4: Add git flows for integration tests

sjs994
sjs994 previously approved these changes May 12, 2022
View reviewed changes
sjs994
sjs994 reviewed May 12, 2022
View reviewed changes
.github/workflows/laravel-integration-tests.yaml Outdated
@@ -0,0 +1,78 @@
on: push
Copy link
Collaborator

Choose a reason for hiding this comment

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

no path filter required ?

@sjs994 sjs994 self-requested a review May 12, 2022 11:40
sjs994
sjs994 reviewed May 12, 2022
View reviewed changes
Copy link
Collaborator

@sjs994 sjs994 left a comment

Choose a reason for hiding this comment

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

Reverting approval

@sjs994 sjs994 dismissed their stale review May 12, 2022 11:43

Some doubts on github workflows

sjs994
sjs994 approved these changes May 12, 2022
View reviewed changes
Copy link
Collaborator

@sjs994 sjs994 left a comment

Choose a reason for hiding this comment

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

Thanks Thiyagu

@sjs994 sjs994 merged commit 09d353b into google:master May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjs994 sjs994 sjs994 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Thiyagu55 @sjs994