Skip to content
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

Environment_Engine: Check for directory in ExportEPW #2787

Merged
merged 3 commits into from
May 23, 2022

Conversation

robinflyman
Copy link
Contributor

Closes #2775

Added a check for a valid directory and a error message if it doesn't exist.

@robinflyman robinflyman added size:S Measured in minutes type:bug Error or unexpected behaviour labels Apr 11, 2022
@robinflyman robinflyman self-assigned this Apr 11, 2022
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

Engine specified in PR title is incorrect, please specify the correct engine that is receiving the changes.

@robinflyman robinflyman changed the title BHoM_Engine: Check for directory in ExportEPW Environment_Engine: Check for directory in ExportEPW Apr 11, 2022
Copy link
Contributor

@jamesramsden-bh jamesramsden-bh left a comment

Choose a reason for hiding this comment

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

Tested in GH. The following failed the new check when they should have passed:

  • A file existing that exactly matches the outputPath variable
  • A folder and a new file, where the folder is already existing

@robinflyman
Copy link
Contributor Author

@BHoMBot check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Apr 25, 2022

@robinflyman to confirm, the following checks are now queued:

  • versioning

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented May 2, 2022

@FraserGreenroyd to confirm, the following checks are now queued:

  • ready-to-merge

There are 4 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented May 2, 2022

@FraserGreenroyd to confirm, the following checks are now queued:

  • ready-to-merge

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check core

@bhombot-ci
Copy link

bhombot-ci bot commented May 2, 2022

@FraserGreenroyd to confirm, the following checks are now queued:

  • core

Copy link
Contributor

@jamesramsden-bh jamesramsden-bh left a comment

Choose a reason for hiding this comment

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

Tested with valid path, path without filename, and path with filename and non-existing directory path. All now behave as expected.

@jamesramsden-bh
Copy link
Contributor

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented May 5, 2022

@jamesramsden-bh to confirm, the following checks are now queued:

  • code-compliance
  • documentation-compliance
  • project-compliance
  • core
  • null-handling
  • serialisation
  • versioning
  • installer

@bhombot-ci
Copy link

bhombot-ci bot commented May 5, 2022

The check versioning has already been run previously and recorded as a successful check. This check has not been run again at this time.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented May 23, 2022

@FraserGreenroyd to confirm, the following checks are now queued:

  • copyright-compliance
  • dataset-compliance

There are 310 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented May 23, 2022

FAO: @FraserGreenroyd
@FraserGreenroyd is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate.

The check they wish to have dispensation on is core.

If you are providing dispensation on this occasion, please reply with:

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 6304860759

@FraserGreenroyd
Copy link
Contributor

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 6304860759

@bhombot-ci
Copy link

bhombot-ci bot commented May 23, 2022

@FraserGreenroyd I have now provided a passing check on reference 6304860759 as requested.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented May 23, 2022

@FraserGreenroyd to confirm, the following checks are now queued:

  • ready-to-merge

There are 306 requests in the queue ahead of you.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented May 23, 2022

@FraserGreenroyd to confirm, the following checks are now queued:

  • ready-to-merge

There are 306 requests in the queue ahead of you.

@FraserGreenroyd FraserGreenroyd merged commit efd0d2e into main May 23, 2022
@FraserGreenroyd FraserGreenroyd deleted the BHoM_Engine-#2775-CheckForDirectory branch May 23, 2022 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S Measured in minutes type:bug Error or unexpected behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BHoM_Engine: Check for valid directory in ExportEPW
3 participants