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: remove most getChalkScope #395

Merged
merged 13 commits into from
Aug 19, 2024
Merged

refactor: remove most getChalkScope #395

merged 13 commits into from
Aug 19, 2024

Conversation

ee7
Copy link
Contributor

@ee7 ee7 commented Aug 5, 2024

Description

Previous refactoring (most recently 6ce7679) worked towards making nearly every get/set occur from the top level. Refactor so that helper procs always get/set from the top level, so that we no longer write getChalkScope() when getting/setting or similar.

Before this PR, there was a lot of getChalkScope() noise:

$ git log --oneline -1
6470eaf4 (HEAD -> main, crashappsec/main) fix: using none log level for chalk exec in interactive shell (#394)
$ git grep 'getChalkScope()' -- '*.nim' | wc -l
265

With this PR, the remaining getChalkScope() outside run_management.nim are mostly nil comparisons:

$ git log --oneline -1
fa1f99b (HEAD -> ee7/reduce-getChalkScope, crashappsec/ee7/reduce-getChalkScope) Merge branch 'main' into ee7/reduce-getChalkScope
$ git grep --ignore-case 'getChalkScope' -- '*.nim' | wc -l
14
$ git grep --break --heading --ignore-case 'getChalkScope' -- '*.nim'
src/chalk_common.nim
484:    if getChalkScope() != nil and attrGet[bool]("chalk_debug"):

src/confload.nim
109:  if tb != "" and getChalkScope() == nil or attrGet[bool]("chalk_debug"):

src/run_management.nim
20:proc getChalkScope*(): AttrScope =
24:  getChalkScope().getObjectOpt(s).isSome()
27:  get[T](getChalkScope(), fqn)
30:  getOpt[T](getChalkScope(), fqn)
33:  getObject(getChalkScope(), fqn)
55:  doAssert attrSet(getChalkScope(), fqn, value, attrType).code == errOk
58:  discard attrLookup(getChalkScope(), fqn.split('.'), ix = 0, op = vlSecDef)

src/selfextract.nim
153:  if getChalkScope() != nil and attrGet[bool]("chalk_debug"):

src/sinks.nim
47:    if (llStr in toLogLevelMap) and getChalkScope() != nil and
153:  if getChalkScope() != nil and attrGet[bool]("chalk_debug"):
185:    attrRoot = if attr != nil: attr else: getChalkScope()
273:      discard setOverride(getChalkScope(), section & ".pinned_cert_file", some(pack(path)))

Refs: #269

Testing

Verify that this is refactoring only.

The below line enables slow tests for this PR:

tests:--slow

@ee7 ee7 changed the title refactor refactor: remove most getChalkScope Aug 5, 2024
@ee7 ee7 force-pushed the ee7/reduce-getChalkScope branch from 83f46b2 to bf939c0 Compare August 5, 2024 18:05
@ee7 ee7 marked this pull request as ready for review August 5, 2024 18:33
@ee7 ee7 requested a review from viega as a code owner August 5, 2024 18:33
@ee7 ee7 requested a review from miki725 August 5, 2024 18:33
@ee7 ee7 merged commit 7e35a57 into main Aug 19, 2024
4 checks passed
@ee7 ee7 deleted the ee7/reduce-getChalkScope branch August 19, 2024 14:42
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.

2 participants