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

Platform independent compilation #172

Merged
merged 17 commits into from
Jan 5, 2024

Conversation

heniu20
Copy link
Contributor

@heniu20 heniu20 commented Dec 20, 2023

No description provided.

pom.xml Outdated
</condition>
<condition property="urlSlashesOsModifier" value="///">
<and>
<os family="windows"></os>
Copy link
Member

Choose a reason for hiding this comment

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

ow windows.... gotta love that.

Funny enough, I'm on windows 11, but all work is done under WSL, so never noticed the problem

pom.xml Outdated
@@ -598,13 +598,42 @@
<stagingProgressTimeoutMinutes>15</stagingProgressTimeoutMinutes>
</configuration>
</plugin>
<plugin>
<artifactId>maven-antrun-plugin</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

can we use maven profiles instead?

ant seems so 1994

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -134,7 +134,8 @@ public void toString_() throws IOException {
BeanSerializer serializer = new BeanSerializer();
serializer.setAddToString(true);
serializer.serialize(type, SimpleSerializerConfig.DEFAULT, new JavaWriter(writer));
assertThat(String.valueOf(writer)).contains(" @Override\n" + " public String toString()");
Copy link
Member

Choose a reason for hiding this comment

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

would a multine string fix this?

assertThat(String.valueOf(writer)).contains("""
 @Override
 public String toString()
""");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, text blocks always use \n

@heniu20 heniu20 requested a review from velo December 30, 2023 12:09
@velo velo merged commit 42a1bad into OpenFeign:master Jan 5, 2024
3 checks passed
@velo
Copy link
Member

velo commented Jan 5, 2024

I made a few changes to eliminate the need of a maven profile and asserting line endings...
Also enable windows buld

Lemme know what you think.

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.

2 participants