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.

Sign up for GitHub

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

Jump to bottom

Refactor active configuration handling #158

Closed
haxscramper opened this issue Jan 16, 2022 · 0 comments · Fixed by #209
Closed

Refactor active configuration handling #158

haxscramper opened this issue Jan 16, 2022 · 0 comments · Fixed by #209
Labels
good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required. refactor Implementation refactor

Comments

@haxscramper
Copy link
Collaborator

Refactor active configuration handling in the options.ConfigRef - right now every single field is placed on the same level, including both mutable non-configuration values (error counter, list of symbols) and active/passive configurations (list of active notes, local and global options).

Field that are used to store explicit configuration values (backend/target/options/globalOptions) need to be factored out into a separate type definition.

  • This would allow treating compiler (at least on conceptual level) as a function proc compile(config: ActiveOptions).
  • This type can be reused in testament to serve as a configuration cell in test matrix
  • Command-line configuration reading can be simplified as well - instead of using a whole ConfigRef object and mutating it in-place, it can simply return ActiveOptions object

Old code should continue to work using getters and setters. Additional upside - this change would provide a single point of tracing for the active configuration and reduce number of direct access to fields (which cannot be debugged with echo, cannot be asserted and so on).

type
+ ActiveOptions = object
+   options*: TOptions

  ConfigRef* {.acyclic.} = ref object 
-   options*: TOptions
+   active: ActiveOptions

+ func options*(conf: ConfigRef): TOptions = conf.active.options
+ proc `options=`*(conf: ConfigRef, opts: TOptions) = conf.active.options = opts

Repeated for all "input" fields: backend, target, options, filenamesOption
@haxscramper haxscramper added good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required. refactor Implementation refactor labels Jan 16, 2022
@haxscramper haxscramper mentioned this issue Jan 16, 2022
Open
haxscramper added a commit to haxscramper/nimskull that referenced this issue Jan 28, 2022
@haxscramper
Refactor active configuration handing
e726324 
- Move active configuration handing into a separate type that should
  contain all the required configuration parameters. Use this type in
  `ConfigRef` module, and use accessor procs for getting and setting values
  of different options, thus removing unguarded access for fields in a
  number of cases.
- Move report kind and category enums into `report_enums` in order to make
  them accessible from the `in_options` - it must be a lightweight
  dependency since it is supposed to be reused by other tools, like
  testament, that have to directly interact with the compiler.

Closes nim-works#158
@haxscramper haxscramper mentioned this issue Jan 28, 2022
Merged
haxscramper added a commit to haxscramper/nimskull that referenced this issue Jan 28, 2022
@haxscramper
Refactor active configuration handing
f947d48 
- Move active configuration handing into a separate type that should
  contain all the required configuration parameters. Use this type in
  `ConfigRef` module, and use accessor procs for getting and setting values
  of different options, thus removing unguarded access for fields in a
  number of cases.
- Move report kind and category enums into `report_enums` in order to make
  them accessible from the `in_options` - it must be a lightweight
  dependency since it is supposed to be reused by other tools, like
  testament, that have to directly interact with the compiler.

Closes nim-works#158
haxscramper added a commit to haxscramper/nimskull that referenced this issue Jan 28, 2022
@haxscramper
Refactor active configuration handing
2d1f4ca 
- Move active configuration handing into a separate type that should
  contain all the required configuration parameters. Use this type in
  `ConfigRef` module, and use accessor procs for getting and setting values
  of different options, thus removing unguarded access for fields in a
  number of cases.
- Move report kind and category enums into `report_enums` in order to make
  them accessible from the `in_options` - it must be a lightweight
  dependency since it is supposed to be reused by other tools, like
  testament, that have to directly interact with the compiler.

Closes nim-works#158
haxscramper added a commit to haxscramper/nimskull that referenced this issue Jan 28, 2022
@haxscramper
Refactor active configuration handing
2788480 
- Move active configuration handing into a separate type that should
  contain all the required configuration parameters. Use this type in
  `ConfigRef` module, and use accessor procs for getting and setting values
  of different options, thus removing unguarded access for fields in a
  number of cases.
- Move report kind and category enums into `report_enums` in order to make
  them accessible from the `in_options` - it must be a lightweight
  dependency since it is supposed to be reused by other tools, like
  testament, that have to directly interact with the compiler.

Closes nim-works#158
haxscramper added a commit to haxscramper/nimskull that referenced this issue Jan 28, 2022
@haxscramper
Refactor active configuration handing
51840bf 
- Move active configuration handing into a separate type that should
  contain all the required configuration parameters. Use this type in
  `ConfigRef` module, and use accessor procs for getting and setting values
  of different options, thus removing unguarded access for fields in a
  number of cases.
- Move report kind and category enums into `report_enums` in order to make
  them accessible from the `in_options` - it must be a lightweight
  dependency since it is supposed to be reused by other tools, like
  testament, that have to directly interact with the compiler.

Closes nim-works#158
haxscramper added a commit to haxscramper/nimskull that referenced this issue Jan 28, 2022
@haxscramper
Refactor active configuration handing
12b60bf 
- Move active configuration handing into a separate type that should
  contain all the required configuration parameters. Use this type in
  `ConfigRef` module, and use accessor procs for getting and setting values
  of different options, thus removing unguarded access for fields in a
  number of cases.
- Move report kind and category enums into `report_enums` in order to make
  them accessible from the `in_options` - it must be a lightweight
  dependency since it is supposed to be reused by other tools, like
  testament, that have to directly interact with the compiler.

Closes nim-works#158
@haxscramper haxscramper moved this to In Progress in Test tooling improvements Jan 28, 2022
@haxscramper haxscramper moved this from In Progress to Todo in Test tooling improvements Jan 28, 2022
This was referenced Jan 28, 2022
Closed
Open
haxscramper added a commit to haxscramper/nimskull that referenced this issue Jan 31, 2022
@haxscramper
Refactor active configuration handing
67c56ec 
- Move active configuration handing into a separate type that should
  contain all the required configuration parameters. Use this type in
  `ConfigRef` module, and use accessor procs for getting and setting values
  of different options, thus removing unguarded access for fields in a
  number of cases.
- Move report kind and category enums into `report_enums` in order to make
  them accessible from the `in_options` - it must be a lightweight
  dependency since it is supposed to be reused by other tools, like
  testament, that have to directly interact with the compiler.

Closes nim-works#158
bors bot added a commit that referenced this issue Feb 6, 2022
@bors @haxscramper
Merge #209
854120b 
209: Refactor active configuration handing r=saem a=haxscramper

- Move active configuration handing into a separate type that should
  contain all the required configuration parameters. Use this type in
  `ConfigRef` module, and use accessor procs for getting and setting values
  of different options, thus removing unguarded access for fields in a
  number of cases.
- Move report kind and category enums into `report_enums` in order to make
  them accessible from the `in_options` - it must be a lightweight
  dependency since it is supposed to be reused by other tools, like
  testament, that have to directly interact with the compiler.

Closes #158

Co-authored-by: haxscramper <haxscramper@gmail.com>
@bors bors bot closed this as completed in 32308c8 Feb 6, 2022
Repository owner moved this from Todo to Done in Test tooling improvements Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required. refactor Implementation refactor
Projects
Test tooling improvements
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor active configuration handing haxscramper/nimskull
1 participant
@haxscramper