forked from adelinosousa/gh-cli-auth
-
Notifications
You must be signed in to change notification settings - Fork 0
2.0.0 #1
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
Merged
Merged
2.0.0 #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ble or the properties file, a static default can be forgotten and can lead to accidentally setting the wrong version, instead enforce the availability of the gradle.publish.version property by default
…and multi-module inclusion
… as it enable users to build on top of the plugin if desired
…t appender can be used for this log, plus set the log level to error
…all other classes abbreviate GitHub to Gh
…improve single responsibility, and encapsulation
…ove error handling, and improve dynamic GH binary collector logic
…ganization for clarity and avoid naming clash
….kts to remove IDE marked errors, and remove empty block for functional test sources
…edded version, and correctly set functional tests as test sources rather than main
…ys have an option to override, improve logging, and encapsulate GhCliAuthProcessor provider creation
…lly make testing with GH CLI plausible
…duplication, and notable improve acceptance test coverage at the integration level!
…ing build process
…'gh.cli.auth.token' for consistency
…m version control
44055b1 to
f7f6574
Compare
f7f6574 to
d0abd79
Compare
d0abd79 to
788cc1a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
This plugin is a fantastic candidate to reduce our settings convention boilerplate for enterprise projects hosted on the GH platform. As such, I have invested some of my free time to improve this project's configurations and implementation details.
I have introduced full backwards compatibility with projects that rely on Gradle properties instead of environment variables, making it an attractive drop-in replacement in established projects for users who want the GH CLI authentication option.
On top of that, I have made it possible to functionally test this plugin with the GH CLI processor by creating a suitable fake. This pushed me to complete the test coverage for acceptance requirements, which adds confidence for buy-ins and adoption opportunities.
Finally, I have fully revised the documentation to ensure its accuracy and alignment with the latest details.
Implementation Details
There are a lot of technical details to cover. I will add some highlights here, and I am more than happy to call for a complete walkthrough and reasoning breakdown.
⚙️ GitHub CLI Auth Processor and Parser
The implementation details for the GH auth highly resembled processor and parser patterns. I have refactored them to be clear of that, gave them better names with complete test coverage. I have also settled for
Ghas a suffix rather thanGitHubfor naming, as I noticed there was mixed usage of the two.⚙️ Gh CLI Auth Base (Abstract Class) to Reduce Duplication!
The GH CLI Auth project and settings plugin share a lot of commonality, so it was natural to create a base class that holds those details for ease of maintenance and understanding of the different functionality between the two. I also added a nice little cache implementation for the GH org and credentials!
🧪 Functional Tests Cross-Testing Coverage
Since both the project and settings plugin share the same base abstract class, I have completed the coverage by crossing which base common features to test between the two. For example, some CLI auth logic might have been covered in the project plugin functional tests, whereas the property logic might've been covered in the settings plugin functional tests.
🧪 How GH CLI Processor Testing Has Been Made Possible?
The secret is this little fake I have written:
The GH CLI's behavior is NOT our responsibility to test (it's not software the project has written); as such, it's acceptable to fake its response, much like stubbing an API.
🐛 Kotlin JVM and Kotlin DSL Were Not Aligned
The project was applying Kotlin JVM version 2.1.20, yet the embedded Kotlin is 2.0.21. This means the following:
plugins { `kotlin-dsl` alias(libs.plugins.kotlin.jvm) }was applying to different Kotlin APIs! This can lead to unexpected conflicts, so I have set the KT version to match what the current Gradle wrapper version embeds.
🐛 Functional Test Setup Configuration Was Incorrect
The
functionalTestsrc was being picked up as non-test sources. Using theJvmTestSuite, I have correctly configured and set up this source to correctly identify the files there as tests (you will see them marked as "green" instead of "blue" from now on in the IDE).A bunch of warnings were sorted out based on the current Gradle version being used. See:
If you need any more details, please let me know!