nextcloud / openapi-extractor

A tool for extracting OpenAPI specifications from Nextcloud source code
https://docs.nextcloud.com/server/latest/developer_manual/client_apis/OCS/ocs-openapi.html
GNU Affero General Public License v3.0
6 stars 2 forks source link

fix: Remove dangerous --continue-on-error flag #152

Closed provokateurin closed 1 month ago

provokateurin commented 1 month ago

While this flag was meant to speed up development, people are starting to abuse it any ignore any errors. They won't even know that the spec isn't complete due to all the errors.

nickvergessen commented 1 month ago

I would keep the option, but still make the result != 0, so that CI complains? But at least you get the full list of your errors

provokateurin commented 1 month ago

I think all the errors that are thrown are important and shouldn't be ignored, except for the missing docs which already have a separate flag that can be used to skip them.

nickvergessen commented 1 month ago

yeah, which is why it should still error the command and on CI. But it's quite annoying if you have to run the command step by step to reach the next error?

My suggestion would be additionally to your current patch:

diff --git a/generate-spec b/generate-spec
index 0780d47..1108a78 100755
--- a/generate-spec
+++ b/generate-spec
@@ -1024,3 +1024,8 @@ foreach ($scopePaths as $scope => $paths) {

        Logger::info('app', 'Generated scope ' . $scope . ' with ' . $pathsCount . ' routes!');
 }
+
+if (Logger::$loggedError) {
+       // Error the run on CI when an error was logged
+       exit(1);
+}
diff --git a/src/Logger.php b/src/Logger.php
index 1af603a..a7cea6a 100644
--- a/src/Logger.php
+++ b/src/Logger.php
@@ -4,6 +4,7 @@ namespace OpenAPIExtractor;

 class Logger {
        public static bool $verbose = false;
+       public static bool $loggedError = false;

        protected static function log(LoggerLevel $level, string $context, string $text): void {
                print(self::format($level, $context, $text));
@@ -33,10 +34,8 @@ class Logger {
                self::log(LoggerLevel::Warning, $context, $text);
        }

-       /**
-        * @throws LoggerException
-        */
        public static function error(string $context, string $text): void {
-               throw new LoggerException(LoggerLevel::Error, $context, $text);
+               self::$loggedError = true;
+               self::log(LoggerLevel::Error, $context, $text);
        }
 }
provokateurin commented 1 month ago

Not necessarily everyone runs it in CI, so the final exit code might be missed. But I guess your suggestion makes sense, then the flag can still be removed.