-
Notifications
You must be signed in to change notification settings - Fork 2
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
Set up build configurations and references for GSA10.1 build #243
Set up build configurations and references for GSA10.1 build #243
Conversation
Tested various old scripts using the GSA adapter, using push/ pull/ update etc, after compiling the 'debug' build. All work well. |
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.
The suggested change can be ignored if you want.
My actual request for changes would be to include an altConfigs.txt
file (see an example here) that would allow BHoMBot to check both configurations in a check core
check (this doesn't impact the installers).
Can I confirm the default build at the moment is still targeting 8_7 (does impact installers) (which is also the purpose of the first code comment).
Otherwise, from a CI perspective I'm happy with this.
public partial class GSA_10_1Adapter | ||
#elif GSA_8_7 | ||
public partial class GSA_8_7Adapter | ||
#else |
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.
What version of GSA will the default GSAAdapter
be targetting?
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.
The default build is targeting GSA 8.7. WHat I did though is that I did not define any constants for the default debug and release builds, so they should still revert to the #else
case. This essentially means that we will have two identical builds. GSAAdapter and GSA8_7Adapter will be doing the exact same thing.
My reasoning for setting it up this way is that I did not want this PR to impact the beta release in any way at all. Plan (which I will document on the issue if we agree with this strategy) is that we remove the GSAAdapter in milestone 4.3, with proper versioning and everything, and only have the versioned builds left. Basically, remove default Debug and Release builds and remove the else statement.
Does that make sense?
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 does make sense.
If we don't change the configs for the installer (which is what we're recommending here, only adding one for core
) then the 4.2 beta will be unchanged, so then doing a full upgrade in 4.3 with versioning is probably a good plan, I'd support that as a roll out.
<3 Co-authored-by: Fraser Greenroyd <[email protected]>
@BHoMBot check required |
@FraserGreenroyd to confirm, the following checks are now queued:
|
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.
Approved with discussion with @IsakNaslundBh
Issues addressed by this PR
Handles initial setup of #242
Test files
Changelog
Additional comments
Should build as before for the previous Debug and Release builds, with GSAAdapter being the resulting class name. Will look into removing this and version GSAAdapter to the 8.7 build after 4.2 beta has been released.
@FraserGreenroyd adding you in for general compliance check. DO not expect you to test the code, just making sure I am not adding anything here that will be in the way for the release. The intention is that only the new builds should be different from before, while the default builds should remain unaffected