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

CI triggered on pull_request event #791

Merged
merged 3 commits into from
Jul 27, 2022
Merged

CI triggered on pull_request event #791

merged 3 commits into from
Jul 27, 2022

Conversation

kelson42
Copy link
Collaborator

New attempt to fix #410 and maybe superseed #409

@kelson42 kelson42 force-pushed the ci_pull_request branch 7 times, most recently from 5730e3b to 75a043c Compare June 26, 2022 18:21
@codecov
Copy link

codecov bot commented Jun 26, 2022

Codecov Report

Merging #791 (b69bf4d) into master (abccd9d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #791   +/-   ##
=======================================
  Coverage   70.65%   70.65%           
=======================================
  Files          53       53           
  Lines        3708     3708           
  Branches     2061     2061           
=======================================
  Hits         2620     2620           
  Misses       1086     1086           
  Partials        2        2           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abccd9d...b69bf4d. Read the comment docs.

@kelson42 kelson42 force-pushed the ci_pull_request branch 2 times, most recently from 85cd684 to 1e19588 Compare July 3, 2022 17:35
@rgaudin
Copy link
Member

rgaudin commented Jul 4, 2022

I don't have GA knowledge in mind to verify what this will do but:

I agree with @mgautierfr in #409 and #410 that “CI” may not be the best name for this workflow.

I'm worried we may end-up with a single check on PR opening and not on subsequent commits inside the PR doc is unclear to me – both for external PR and internal ones.

@kelson42
Copy link
Collaborator Author

kelson42 commented Jul 4, 2022

@rgaudin Yes the doc is really unclear about new commits... I worried about the same. But, it works like expected.

@kelson42
Copy link
Collaborator Author

kelson42 commented Jul 4, 2022

I agree with @mgautierfr in #409 and #410 that “CI” may not be the best name for this workflow.

"Continuous integration (CI) is the practice of automating the integration of code changes from multiple contributors into a single software project. It’s a primary DevOps best practice, allowing developers to frequently merge code changes into a central repository where builds and tests then run. Automated tools are used to assert the new code’s correctness before integration." https://www.atlassian.com/continuous-delivery/continuous-integration

This is exactly what this workflow does: checking code to secure it can be merged (and checking master to secure it is merged correctly and still run). I hardly see what could be a better name?

@kelson42 kelson42 merged commit 98bcf8a into master Jul 27, 2022
@kelson42 kelson42 deleted the ci_pull_request branch July 27, 2022 19:28
mgautierfr added a commit that referenced this pull request Nov 30, 2022
* [API Break] Remove wrapper around libzim (@mgautierfr #789)
* Allow kiwix-serve to use custom resource files (@veloman-yunkan #779)
* Properly handle searchProtocolPrefix when rendering search result (@veloman-yunkan #823)
* Prevent search on multi language content (@veloman-yunkan #838)
* Use new `zim::Archive::getMediaCount` from libzim (@mgautierfr #836)
* Catalog:
 - Include tags in free text catalog search (@veloman-yunkan #802)
 - Illustration's url is based on book's uuid (@veloman-yunkan #804)
 - Cleanup of the opds-dumper (@veloman-yunkan #829)
 - Allow filtering of catalog content using multiple languages (@veloman-yunkan #841)
 - Make opds-dumper respect the namemapper (@mgautierfr #837)
* Server:
 - Correctly handle `\` in suggestion json generation (@veloman-yunkan #843)
 - Better http caching (@veloman-yunkan #833)
 - Make `/suggest` endpoint thread-safe (@veloman-yunkan #834)
 - Better redirection of main page (@veloman-yunkan #827)
 - Remove jquery (@mgautierfr @juuz0 #796)
 - Better Viewer of zim content :
   . Introduce `/content` endpoints (@veloman-yunkan #806)
   . Switch to iframe based content viewer (@veloman-yunkan #716)
 - Optimised design of the welcome page:
   . Alignement (@juuz0 @kelson42 #786)
   . Exit download modal on pressing escape key (@Juzz0 #800)
   . Add favicon for different devices (@Juzz0 #805)
   . Fix auto hidding of the toolbar (@veloman-yunkan #821)
   . Allow user to filter books by tags in the front page (@juuz0 #711)
* CI :
  - Trigger CI on pull_request (@kelson42 #791)
  - Drop Ubuntu Impish packaging (@legoktm #825)
  - Add Ubuntu Kinetic packaging (@legoktm #801)
* Testing:
  - Test ICULanguageInfo (@veloman-yunkan #795)
  - Introduce fake `test` language to test i18n (@veloman-yunkan #848)
* Fix documentation (@kelson42 #816)
* Udpate translation (#787 #839 #847)
@mgautierfr mgautierfr mentioned this pull request Nov 30, 2022
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.

CI should be run on pull_request event
2 participants