-
Notifications
You must be signed in to change notification settings - Fork 210
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
Harmonise wildcards #1223
Harmonise wildcards #1223
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, this looks nice for me; few minor comments.
test/config.custom.yaml
Outdated
@@ -3,7 +3,7 @@ | |||
# SPDX-License-Identifier: CC0-1.0 | |||
|
|||
### CHANGES TO CONFIG.TUTORIAL.YAML ### | |||
version: 0.4.1 | |||
version: 0.4.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes are probably a refuse? It applies also to the following yaml files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about fixing a typo: the current version of the config is 0.4.2
, and it should be probably the same across the scripts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is probably a refuse maybe; hadn't the number to match the version of the repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is probably a refuse maybe; hadn't the number to match the version of the repo?
Agree that it would be best when there is a match between the configuration and the release versions. But what is we introduce changes into the config when the repo version remains the same? We have discussed the idea to use sub-versions of the config to track (e.g. 0.4.1.1
) that but agreed that it's against semantic versioning. Ideally, it would be great to have a release every time when the config changes, but that is not exactly realistic as far as we don't have an automated release process
Happy to discuss at the next convenience 🙂
Also this one can be merged right? @ekatef ? |
I think yes 🙂 |
mmm... conflicts :'( would you mind fixing them? |
Thanks a lot for the reminder 🙂 Done! |
Closes #1197
Changes proposed in this Pull Request
Arguments of
mock_snakemake
have been updated using parameters of the sector-coupled test config (config.test1.yaml renamed into config.sector.yaml)Checklist
envs/environment.yaml
anddoc/requirements.txt
.config.default.yaml
andconfig.tutorial.yaml
.test/
(note tests are changing the config.tutorial.yaml)doc/configtables/*.csv
and line references are adjusted indoc/configuration.rst
anddoc/tutorial.rst
.doc/release_notes.rst
is amended in the format of previous release notes, including reference to the requested PR.