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

Add coverage to workflow #51 #52

Merged
merged 7 commits into from
Mar 25, 2024
Merged

Add coverage to workflow #51 #52

merged 7 commits into from
Mar 25, 2024

Conversation

lorenzocelli
Copy link
Contributor

@lorenzocelli lorenzocelli commented Feb 24, 2024

@lorenzocelli lorenzocelli linked an issue Feb 24, 2024 that may be closed by this pull request
@lorenzocelli
Copy link
Contributor Author

lorenzocelli commented Feb 24, 2024

With this action it is currently not possible to include the badge in the readme.

@siliconlad do we want the coverage to fail under a certain threshold?

@cadifyai cadifyai deleted a comment from github-actions bot Feb 24, 2024
@siliconlad
Copy link
Member

siliconlad commented Feb 25, 2024

With this action it is currently not possible to include the badge in the readme.

Is it silly to get the bot to overwrite the badge in the README every time (or on merge)?

@siliconlad
Copy link
Member

@siliconlad do we want the coverage to fail under a certain threshold?

Is there a reason not to set it at 100%?

@lorenzocelli
Copy link
Contributor Author

Is there a reason not to set it at 100%?

I don't see any. I will have it fail then

@lorenzocelli
Copy link
Contributor Author

Is it silly to get the bot to overwrite the badge in the README every time (or on merge)?

I don't think it is worth the effort, but if we really want the badge we can do it

Copy link

Code Coverage

Package Line Rate Complexity Health
. 98% 0
Summary 98% (251 / 257) 0

1 similar comment
Copy link

Code Coverage

Package Line Rate Complexity Health
. 98% 0
Summary 98% (251 / 257) 0

@siliconlad
Copy link
Member

Is it silly to get the bot to overwrite the badge in the README every time (or on merge)?

I don't think it is worth the effort, but if we really want the badge we can do it

Let's leave it then 👍

@siliconlad
Copy link
Member

Do we need the bot vs just a job that fails? I wonder how annoying the bot will become.

@lorenzocelli
Copy link
Contributor Author

Let's get rid of the bot 🤖🔫

@lorenzocelli
Copy link
Contributor Author

The non-tested code we have is the following. In parameter_schema.py:

class ParameterSchema:

    @staticmethod
    def matches(parameter: Parameter) -> bool:
        raise NotImplementedError()  # <---- this line

    def _get_type(self) -> Union[str, Parameter.empty]:
        return Parameter.empty  # <---- this line

Maybe it can be solved with abstract classes.
In schema.py:

# ---- this method ----
def SaveGPTEnabled(module: ModuleType, path: str, schema_type: SchemaType = SchemaType.API):
    schemas = FindGPTEnabledSchemas(module, schema_type)
    json.dump(schemas, open(path, "w"))

# ...

class _GPTEnabled:
    # ...

    # ---- this method ----
    def gpt_enabled(self) -> bool:
        return True

# ...

class FunctionSchema:

    # ...

    def _get_parameters_schema(self, schema_type: SchemaType) -> dict:
        schema = {"type": "object"}
        
        if not self.parameter_schemas and schema_type == SchemaType.API:
            # Skip properties
            return schema  # <---- this line

@siliconlad siliconlad marked this pull request as ready for review March 25, 2024 16:34
@siliconlad
Copy link
Member

Lowered the threshold to 95%.

@siliconlad siliconlad merged commit 441968f into main Mar 25, 2024
5 checks passed
@siliconlad siliconlad deleted the enhancement/add-coverage branch March 25, 2024 16:35
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.

Add Coverage to workflow
2 participants