obsproject / obs-studio

OBS Studio - Free and open source software for live streaming and screen recording
https://obsproject.com
GNU General Public License v2.0
60.39k stars 7.99k forks source link

spich_refactor #11458

Closed spikovich closed 3 weeks ago

spikovich commented 3 weeks ago

Description This pull request introduces several improvements to the build workflow and refactors the aja-card-manager.cpp file in the AJA plugin. The key changes are:

Workflow Improvements:

Added a GitHub Actions Workflow: Introduced a new GitHub Actions workflow (cmake-single-platform.yml) for building and testing the project using CMake on a single platform (Ubuntu). This automates the build and test processes for pull requests and commits to the master branch. CMake Version Parsing Enhancements:

Improved Version Parsing Logic: Updated cmake/common/versionconfig.cmake to enhance version parsing from git describe output. Added checks to ensure the parsed version has at least three components (major, minor, patch) and included error messages for failed parsing. Code Refactoring:

Refactored aja-card-manager.cpp: Improved readability and performance by: Removing redundant code and unnecessary variable declarations. Using static_cast for type safety. Simplifying loops with consistent increment operators and variable types. Streamlining logic in the Initialize and AcquireChannel methods. Motivation and Context Automated Build and Test Workflow: Automating the build and test processes ensures consistency and reliability in the development workflow. It helps catch integration issues early and provides immediate feedback to contributors. Reliable Version Parsing: Accurate version parsing is essential for release management and ensuring compatibility. Enhanced error checking prevents build failures due to incorrect version information. Improved Code Quality: Refactoring the AJA plugin codebase enhances maintainability, making it easier for developers to understand and contribute. It also improves performance by eliminating unnecessary operations. How Has This Been Tested? Workflow Testing:

Triggered the new GitHub Actions workflow through test commits and pull requests. Verified that the workflow checks out the code, configures CMake, builds the project, and runs tests successfully. CMake Script Testing:

Ran the CMake configuration in different environments, including ones with and without annotated git tags. Confirmed that the version parsing logic correctly extracts version numbers and handles errors gracefully. AJA Plugin Testing:

Built the refactored aja-card-manager.cpp on systems equipped with AJA hardware. Conducted functional tests to ensure that card initialization, channel acquisition, and interrupt handling work as expected. Verified that there are no regressions in AJA input/output functionalities. Types of changes Code cleanup (non-breaking change which makes code smaller or more readable) Performance enhancement (non-breaking change which improves efficiency) Continuous Integration (addition of a new GitHub Actions workflow) Checklist: My code has been run through clang-format. I have read the contributing document. My code is not on the master branch. The code has been tested. All commit messages are properly formatted and commits squashed where appropriate. [n/a] I have included updates to all appropriate documentation. Note: No documentation updates were necessary as this PR does not introduce changes that affect user-facing documentation.

Fenrirthviti commented 3 weeks ago

This feels like it might have been opened prematurely.

Please update the commit titles, the PR description, and fix the formatting in the PR template. In the future mind that you don't break the formatting of the PR template. It makes it difficult to read and understand.

spikovich commented 3 weeks ago

This feels like it might have been opened prematurely.

Please update the commit titles, the PR description, and fix the formatting in the PR template. In the future mind that you don't break the formatting of the PR template. It makes it difficult to read and understand.

Yeah im sorry, im new here. i try my best