Skip to content

Fix Issue #295#296

Merged
davideicardi merged 2 commits intodynamicexpresso:masterfrom
Pentiva:patch-1
Oct 3, 2023
Merged

Fix Issue #295#296
davideicardi merged 2 commits intodynamicexpresso:masterfrom
Pentiva:patch-1

Conversation

@Pentiva
Copy link
Copy Markdown
Contributor

@Pentiva Pentiva commented Sep 14, 2023 •
edited
Loading

This fixes issue #295 and does not fail any other tests, so I think this is probably the correct solution.
Whether this should be extended/limited in some way based on InterpreterOptions.LateBindObject, I'm not sure.

@davideicardi
Copy link
Copy Markdown
Member

davideicardi commented Sep 21, 2023

Thank you @Pentiva!
I see tests are failing now because Path is not defined. I suggest to use just a static string.

Regarding the solution I don't see problem, but not sure if there is some edge case that I'm not aware. What do you think @metoule ?

@Pentiva
Added Missing Using Statements
8746c32 
Switched to string.Concat because Path.Combine changes might differ per OS
@Pentiva
Copy link
Copy Markdown
Contributor Author

Pentiva commented Sep 21, 2023

Sorry about that, I was using the online IDE github.dev for the first time and didn't notice it didn't add the usings automatically.
I think I'll stick to using my local computer for now.

davideicardi
davideicardi approved these changes Oct 3, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@davideicardi davideicardi left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@davideicardi davideicardi merged commit 2696232 into dynamicexpresso:master Oct 3, 2023
This was referenced Nov 12, 2024
Closed
Closed
metoule added a commit to metoule/DynamicExpresso that referenced this pull request Nov 16, 2024
@metoule
Revert changes done in dynamicexpresso#296
1675a6d
@metoule metoule mentioned this pull request Nov 16, 2024
Merged
metoule added a commit that referenced this pull request Nov 18, 2024
@metoule
Revert changes introduced in #296 (#333)
546a0cb 
* Fix format

* Simplify ExpressionUtils.PromoteExpression

* Revert changes done in #296
@metoule metoule mentioned this pull request Nov 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davideicardi davideicardi davideicardi approved these changes

@metoule metoule Awaiting requested review from metoule metoule is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Pentiva @davideicardi