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

chore/STREAMPIPES-590 #147

Merged
merged 5 commits into from
Nov 22, 2022
Merged

chore/STREAMPIPES-590 #147

merged 5 commits into from
Nov 22, 2022

Conversation

bossenti
Copy link
Contributor

Purpose

Add licencse headers to all strings.en files.

Remarks

Fixes: STREAMPIPES-590

@tenthe
Copy link
Contributor

tenthe commented Nov 19, 2022

Hi Tim,
great job with adding all the license headers.
Did this directly work or did you also have to change something in the java implementation when reading the files?

I think we can now also remove the from the pom.xml file (line 1719). The language files are currently excluded from the checks of the rat plugin.

@bossenti
Copy link
Contributor Author

@tenthe thats a great suggestion!
The strings.* files are now covered by the rat plugin and it directly discovered two files that I missed ;)

Fortunately, there was no need to adapt how we reed the files.
I've tested it with one or two examples.

@tenthe
Copy link
Contributor

tenthe commented Nov 19, 2022

Ok great.
Do you know why the e2e tests failed before? Is it related to the changes of the PR?

@bossenti
Copy link
Contributor Author

They should now go through, there was a silly mistake 😵‍💫
3560522

But the rat plugin is still failing, I will investigate
Maybe we should exclude the artifact strings instead of adding the header? 🤔

@dominikriemer
Copy link
Member

Rat fails due to some Maven error...there seems to be some mirror down, just wait a few hours

Connect to repo.maven.apache.org:443 [repo.maven.apache.org/151.101.40.215] failed: Connection timed out (Connection timed out) -> [Help 1]

Copy link
Member

@dominikriemer dominikriemer left a comment

Choose a reason for hiding this comment

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

@bossenti Thank you very much for adding these comments.
I am not sure about the comment syntax: As these files are basically properties files, shouldn't this be a "#" instead of HTML syntax?

@bossenti
Copy link
Contributor Author

Hm, good point @dominikriemer
I was also tinkering between these two options... I can also switch to # comment style
Is there any advice from the Java Library we are using for reading the resource files?

@dominikriemer
Copy link
Member

Yes, I think the Java convention proposes hash or explanation mark:

https://en.m.wikipedia.org/wiki/.properties

Thanks!

@bossenti
Copy link
Contributor Author

Header style is changed to #

@bossenti bossenti merged commit d0300e2 into dev Nov 22, 2022
@bossenti bossenti deleted the chore/STREAMPIPES-590 branch November 22, 2022 19:47
@bossenti bossenti linked an issue Nov 27, 2022 that may be closed by this pull request
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.

Add Apache header to "strings.en" files in resources
3 participants