Skip to content

Separate build config from base kw config #616

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
Closed
wants to merge 3 commits into from
Closed

Separate build config from base kw config #616

wants to merge 3 commits into from

Conversation

isinyaaa
Copy link
Contributor

This is a change requested by @rodrigosiqueira in PR #543 that was quite large, so I preferred to split it. Note: this is based on #615.

@isinyaaa isinyaaa mentioned this pull request May 26, 2022
Closed
@rodrigosiqueira
Copy link
Collaborator

Hi @isinyaaa ,
I recently created etc/init_templates/ where I maintain all the default configs. Please, update your change to put the new build files template in that folder. Do not forget to update kw init as well.

@isinyaaa isinyaaa force-pushed the separate-build-config branch from dedc45b to af85953 Compare May 27, 2022 19:43
@rodrigosiqueira
Copy link
Collaborator

@isinyaaa could you check the unit test failure?

@isinyaaa isinyaaa force-pushed the separate-build-config branch from af85953 to 687d6b8 Compare June 3, 2022 22:49
@kwy95
Copy link
Collaborator

kwy95 commented Jun 4, 2022

It seems that the kworkflow_template.config is not where the setup.sh file expects it to be, that's why the CI is failing.

@isinyaaa isinyaaa force-pushed the separate-build-config branch from 687d6b8 to a6e2ec8 Compare June 5, 2022 23:55
isinyaaa added 2 commits June 5, 2022 21:28
@isinyaaa
setup.sh: remove deprecated setup_config_files function
c2577df 
This function has been superseded by the one in `kw init`, making it
always fail kw's set up tests in the CI needlessly.

Signed-off-by: Isabella Basso <isabbasso@riseup.net>
@isinyaaa
src: config_loader: Skip empty lines when parsing config files
2686726 
This should provide a small performance improvement as we're no longer
processing empty lines in the parser.

Signed-off-by: Isabella Basso <isabbasso@riseup.net>
@isinyaaa isinyaaa force-pushed the separate-build-config branch from a6e2ec8 to e56d8cc Compare June 6, 2022 00:28
@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2022 •
edited
Loading

Codecov Report

Merging #616 (d538327) into unstable (ff78297) will decrease coverage by 0.09%.
The diff coverage is 70.27%.

@@             Coverage Diff              @@
##           unstable     #616      +/-   ##
============================================
- Coverage     73.84%   73.74%   -0.10%     
============================================
  Files            36       36              
  Lines          5574     5645      +71     
============================================
+ Hits           4116     4163      +47     
- Misses         1458     1482      +24
Impacted Files Coverage Δ
src/kw_config_loader.sh 51.38% <54.00%> (-1.95%) ⬇️
src/build.sh 86.33% <78.26%> (-4.40%) ⬇️
src/config_manager.sh 79.65% <100.00%> (ø)
src/deploy.sh 67.20% <100.00%> (+0.10%) ⬆️
src/init.sh 84.42% <100.00%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff78297...d538327. Read the comment docs.

@isinyaaa isinyaaa force-pushed the separate-build-config branch from e56d8cc to a69ca16 Compare June 6, 2022 01:08
@isinyaaa
build_template: Add a separate config file for build options
d538327 
This adds a separate config file for build options, so that they can be
separated as needed, i.e. per-environment (see kworkflow#537).

Signed-off-by: Isabella Basso <isabbasso@riseup.net>
@isinyaaa isinyaaa force-pushed the separate-build-config branch from a69ca16 to d538327 Compare June 6, 2022 01:10
@rodrigosiqueira
Copy link
Collaborator

rodrigosiqueira commented Jun 7, 2022 •
edited
Loading

Hi @isinyaaa , I did not complete my review yet, but I noticed some bugs.

If you try:

kw init --template

you will see an error like this:

find: ‘/home/siqueira/.config/kw/init_templates’: No such file or directory

If you try:

`kw init --template=rpi4-raspbian-64-cross-x86-arm`

You will see something like this:

We will stop supporting kworkflow.config in the kernel root directory in favor of using a .kw/ directory.
Do you want to migrate to the new configuration file approach? (Recommended) [y/N]: y
It looks like you already have a kw config file. Do you want to overwrite it? [y/N]: y
No such: /home/siqueira/.config/kw/init_templates/:rpi4-raspbian-64-cross-x86-arm/kworkflow_template.config

Could you take a look at that?

@rodrigosiqueira
Copy link
Collaborator

I fixed the issue that I mentioned before and merged this change in the unstable branch. Thanks.

@isinyaaa isinyaaa deleted the separate-build-config branch June 12, 2022 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@isinyaaa @rodrigosiqueira @kwy95 @codecov-commenter