Skip to content

Add ability to use branches of other users#82

Merged
SeanBryan51 merged 5 commits intomasterfrom
61-use-branches-of-other-users
May 31, 2023
Merged

Add ability to use branches of other users#82
SeanBryan51 merged 5 commits intomasterfrom
61-use-branches-of-other-users

Conversation

@SeanBryan51
Copy link
Copy Markdown
Collaborator

The user should be able to checkout branches of other users so that test suites can be reproduced by any user.

This change lets the user specify the path relative to the SVN root of the CABLE repository for each branch by specifying the path key in the configuration file. Remove the trunk and share_branch keys from the configuration file as this information is contained within path.

This change repurposes the name key for each branch to be an alias used internally by benchcab. This lets the user specify the directory name when checking out the branch, that is, name is the optional PATH argument to svn checkout`. This is useful in the case that we have a collision in branch names when checking out multiple branches.

Remove the -M PBS argument for sending emails in the job script as the default behaviour is sufficient.

Remove the user key as this is now redundant.

Update unit tests for check_config().

Fixes #61

The user should be able to checkout branches of other users so that test
suites can be reproduced by any user.

This change lets the user specify the path relative to the SVN root of
the CABLE repository for each branch by specifying the `path` key in the
configuration file. Remove the `trunk` and `share_branch` keys from the
configuration file as this information is contained within `path`.

This change repurposes the `name` key for each branch to be an alias
used internally by benchcab. This lets the user specify the directory
name when checking out the branch, that is, `name` is the optional
`PATH` argument to svn checkout`. This is useful in the case that we
have a collision in branch names when checking out multiple branches.

Remove the `-M` PBS argument for sending emails in the job script as the
default behaviour is sufficient.

Remove the `user` key as this is now redundant.

Update unit tests for `check_config()`.

Fixes #61
@SeanBryan51 SeanBryan51 linked an issue May 18, 2023 that may be closed by this pull request
@SeanBryan51 SeanBryan51 requested a review from ccarouge May 18, 2023 00:59
SeanBryan51 added a commit to CABLE-LSM/bench_example that referenced this pull request May 18, 2023
@SeanBryan51
Copy link
Copy Markdown
Collaborator Author

SeanBryan51 commented May 18, 2023

TODO

  • If we are repurposing the name key in the configuration file to be an alias. This should be an optional key. By default we should use the base name of the path key.

Copy link
Copy Markdown
Member

@ccarouge ccarouge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's good. But you forgot to update the documentation.

Comment thread benchcab/job_script.py
#PBS -q normal
#PBS -P {project}
#PBS -j oe
#PBS -M {email_address}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead should we have:

#PBS -m e

This will send an email at completion of the script whether it succeeds or fails.

@ccarouge
Copy link
Copy Markdown
Member

ccarouge commented May 19, 2023

Do we want to make name optional? And use the stem of path if not provided?

Send email to user by default when job terminates.
Specifying the `name` as an alias is only useful when we have a name
collision between branches which is rare. By default, set the `name`
key to be the base name of the `path` key.
SeanBryan51 added a commit to CABLE-LSM/bench_example that referenced this pull request May 19, 2023
@SeanBryan51 SeanBryan51 requested a review from ccarouge May 19, 2023 03:15
@SeanBryan51 SeanBryan51 merged commit 7b8300a into master May 31, 2023
@SeanBryan51 SeanBryan51 deleted the 61-use-branches-of-other-users branch May 31, 2023 02:30
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.

Use branches of other users

2 participants