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

refactor: begin to avoid autogenerated getters #235

Merged
merged 13 commits into from
Mar 28, 2024
Merged

Conversation

ee7
Copy link
Contributor

@ee7 ee7 commented Mar 7, 2024

Issue

#214

Description

To ease the transition to con4m v2, we want to reduce our usage of autogenerated getters in src/c4autoconf.nim.

The all-encompassing object that c4autoconf.nim defines is ChalkConfig, which currently contains:

type ChalkConfig* = ref object
  `@@attrscope@@`*: AttrScope
  [...]
  attestationConfig*: AttestationConfig
  [...]
  extractConfig*: ExtractConfig
  dockerConfig*: DockerConfig
  execConfig*: ExecConfig
  loadConfig*: LoadConfig
  envConfig*: EnvConfig
  srcConfig*: SrcConfig
  cloudProviderConfig*: CloudProviderConfig
  [...]

As a starting point, try to refactor away all accesses of the fields that end in Config (which are shown above).

That is, with this PR, the following command no longer returns any matches:

git grep 'chalkConfig\.\w*Config\.'

I'll split this work across a few PRs. Another PR will be for all the fields that are of a simple built-in type.

In con4m v2, we'll be using this proc:

proc lookup*[T](ctx: RuntimeState, key: string): Option[T]

like

runtime.lookup("foo.bar").get()

so this PR makes it easier for us to refactor to that later.

This PR adds a get and getOpt in config.nim that calls the get proc in con4m so we can write:

get[T](chalkConfig, "foo.bar")

or with the :T syntax (edit: this was vetoed):

chalkConfig.get[:T]("foo.bar")

Refs: #214

Testing

Hopefully the existing tests are sufficient.

@ee7 ee7 force-pushed the ee7/avoid-c4autoconf-getters branch from 35cfb59 to c79c8b7 Compare March 7, 2024 19:34
@ee7 ee7 marked this pull request as ready for review March 11, 2024 15:48
@ee7 ee7 requested a review from viega as a code owner March 11, 2024 15:48
@ee7
Copy link
Contributor Author

ee7 commented Mar 27, 2024

@miki725 Please review :)

@ee7 ee7 merged commit 66a5bb4 into main Mar 28, 2024
2 checks passed
@ee7 ee7 deleted the ee7/avoid-c4autoconf-getters branch March 28, 2024 15:47
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.

3 participants