Skip to content

Conversation

@nivcertora
Copy link
Contributor

@nivcertora nivcertora commented Apr 8, 2025

Copy link
Contributor

@yoav-el-certora yoav-el-certora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing 2 cases from the ticket

@nivcertora
Copy link
Contributor Author

You are missing 2 cases from the ticket

This case is test not an example:
If there the base config has the attribute override_base_config, return an appropriate error.

Copy link
Contributor

@yoav-el-certora yoav-el-certora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from my point of view.
@urikirsh Do we want to present anything else here?

Copy link
Contributor

@yoav-el-certora yoav-el-certora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nivcertora @rahav-certora Is there a reason why base.conf structure is different than new_fields.conf?

i.e all keys in base.conf and invalid_base.conf are wrapped with "" (like jsons) and new_fields.conf, override_fields.conf not?

@nivcertora
Copy link
Contributor Author

@nivcertora @rahav-certora Is there a reason why base.conf structure is different than new_fields.conf?

i.e all keys in base.conf and invalid_base.conf are wrapped with "" (like jsons) and new_fields.conf, override_fields.conf not?

Fixed

Co-authored-by: urikirsh <38188877+urikirsh@users.noreply.github.com>
@nivcertora nivcertora requested a review from urikirsh April 10, 2025 17:51
Copy link
Contributor

@urikirsh urikirsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing, looks good

@rahav-certora
Copy link
Contributor

@yoav-el-certora - according to the JSON specifications keys need to be in double quotes. Recently I see more and more people not using them, it is allowed in JavaScript but not in JSON, but it works

@yoav-el-certora
Copy link
Contributor

@yoav-el-certora - according to the JSON specifications keys need to be in double quotes. Recently I see more and more people not using them, it is allowed in JavaScript but not in JSON, but it works

This is exactly why I asked to change this here

@nivcertora nivcertora merged commit b4f8002 into cli-beta Apr 14, 2025
@nivcertora nivcertora deleted the niv/CERT-8690-Config-inheritance branch April 14, 2025 15:24
yoav-el-certora added a commit that referenced this pull request May 18, 2025
* Niv/cert 8248 revert example (#155)

* CERT 8248 Add Revert Example

* Update README

* Address Christiane review

* Update example based on Nurit Review

* Clean

* Update config.yml (#160)

Co-authored-by: yoav-el-certora <122207807+yoav-el-certora@users.noreply.github.com>

* Example ready

* christiane cr

* Code reviews

* Config inheritance Example (#168)

* Config inheritance Example

* Update example based on review

* CERT add invalid base conf

* Update base.conf

* CR comment

* Update CVLByExample/ConfInheritance/invalid_base.conf

Co-authored-by: urikirsh <38188877+urikirsh@users.noreply.github.com>

* Fix invalid example

---------

Co-authored-by: urikirsh <38188877+urikirsh@users.noreply.github.com>

* Niv/cert 8958 add require reasoning (#172)

* Add reasoning for require example

* Address Nurit comments

---------

Co-authored-by: Niv vaknin <122722245+nivcertora@users.noreply.github.com>
Co-authored-by: liav-certora <114004726+liav-certora@users.noreply.github.com>
Co-authored-by: liav-certora <liav@certora.com>
Co-authored-by: urikirsh <38188877+urikirsh@users.noreply.github.com>
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.

5 participants