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

KDB_DB_SPEC improvements #2368

Merged
merged 8 commits into from Feb 7, 2019

Conversation

Projects
None yet
2 participants
@kodebach
Copy link
Contributor

kodebach commented Feb 5, 2019

Closes #2352 (we only use KDB_DB_SPEC now)
Closes #1001 (~ is now supported)
Closes #1000 (KDB_DB_SYSTEM must now be absolute)

@markus2330 please review my pull request

@kodebach kodebach self-assigned this Feb 5, 2019

@kodebach

This comment has been minimized.

Copy link
Contributor Author

kodebach commented Feb 6, 2019

Travis again killed the job while uploading the cache after everything was successful, so I consider this "ready to merge".

@markus2330
Copy link
Contributor

markus2330 left a comment

Looks nice, two little things.





# empty lines up to 1000 so that line numbers in the resulting scripts are more useful

This comment has been minimized.

@markus2330

markus2330 Feb 6, 2019

Contributor

It seems like this was not done here.

The idea is that resulting line numbers simply need to be subtracted by 1000, which is much easier to do than to subtract with 373.

You might want to add comment lines, so that the code formatter does not removes them.

This comment has been minimized.

@kodebach

kodebach Feb 6, 2019

Author Contributor

The line after @INCLUDE_COMMON@ in the original file, i.e. line 2, should now be line 1002 in the output file.


# process KDB_DB_SPEC so it is available everywhere
if (IS_ABSOLUTE "${KDB_DB_SPEC}")
set (BUILTIN_SPEC_FOLDER "${KDB_DB_SPEC}")

This comment has been minimized.

@markus2330

markus2330 Feb 6, 2019

Contributor

Do we actually need BUILTIN_SPEC_FOLDER? Why not directly modify KDB_DB_SPEC as done with KDB_DB_SYSTEM?

@kodebach

This comment has been minimized.

Copy link
Contributor Author

kodebach commented Feb 6, 2019

@markus2330 is there a problem in the Jenkins build or can we ignore that?

@markus2330

This comment has been minimized.

Copy link
Contributor

markus2330 commented Feb 6, 2019

jenkins build libelektra please

@markus2330

This comment has been minimized.

Copy link
Contributor

markus2330 commented Feb 6, 2019

I do not know what was wrong with Jenkins, better we try to build it once more.

@markus2330 markus2330 merged commit a1c8e93 into ElektraInitiative:master Feb 7, 2019

2 checks passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@markus2330

This comment has been minimized.

Copy link
Contributor

markus2330 commented Feb 7, 2019

Thank you for the nice PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment