Skip to content

Conversation

@HenkMutsaerts
Copy link
Member

Linked issue

Closes #845 (partly)

How to test

See header of xASL_system. We should be able to merge this now, and improve this function later (according to #845), so then the other Wishlist contents from #845 can then be moved to a new issue :)

Comments

Optional: add helpful comments for the reviewers here

@HenkMutsaerts HenkMutsaerts linked an issue Sep 18, 2021 that may be closed by this pull request
Copy link
Contributor

@MichaelStritt MichaelStritt left a comment

Choose a reason for hiding this comment

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

@HenkMutsaerts: code looks good to me and I really don't like to be the guy who says this, but we agreed during the v1.8.0 release to either show examples of tests you did for the changes or to add a code example on how to test it. This will help us to have a less stressful release process next time.

Besides that: great header, looks awesome and makes the docu process a lot easier.

@MichaelStritt MichaelStritt added the feature New feature, enhancement or request label Sep 18, 2021
@HenkMutsaerts
Copy link
Member Author

Thanks! Good idea, do you something like :

To test this PR:

  • run xASL_system with the examples shown in its header
  • run xASL_qc_TestExploreASL in Linux and Windows, with the option for git pulling enabled. Already tested in macOS

@MichaelStritt
Copy link
Contributor

MichaelStritt commented Sep 20, 2021

Thanks! Good idea, do you something like :

To test this PR:

  • run xASL_system with the examples shown in its header

  • run xASL_qc_TestExploreASL in Linux and Windows, with the option for git pulling enabled. Already tested in macOS

Exactly, something like this.

I tested these cases:

>> xASL_system('echo hello');
hello 
>> [Result1, Result2] = xASL_system('git fetch');
>> [Result1, Result2] = xASL_system('git pull');
Already up to date.
>> [Result1, Result2] = xASL_system('git status');
On branch feature-#845_xASL_system
Your branch is up to date with 'origin/feature-#845_xASL_system'.

nothing to commit, working tree clean

That stuff seems to work fine 👍

@MichaelStritt MichaelStritt self-requested a review September 20, 2021 10:40
Copy link
Contributor

@MichaelStritt MichaelStritt left a comment

Choose a reason for hiding this comment

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

Had to do some minor changes to make it work on windows as well 👍

@MichaelStritt MichaelStritt merged commit 741f951 into develop Sep 20, 2021
@MichaelStritt MichaelStritt deleted the feature-#845_xASL_system branch September 20, 2021 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature, enhancement or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

xASL_system

3 participants