Skip to content

Make JavaScript and XML errors non-TeamCity errors; Update JavaScript language level to ES6 in IntelliJ settings#7541

Merged
gianm merged 3 commits intoapache:masterfrom
metamx:non-teamcity-errors
Apr 25, 2019
Merged

Make JavaScript and XML errors non-TeamCity errors; Update JavaScript language level to ES6 in IntelliJ settings#7541
gianm merged 3 commits intoapache:masterfrom
metamx:non-teamcity-errors

Conversation

@leventov
Copy link
Copy Markdown
Member

Continue efforts to try to resolve problems that prevent TeamCity from updating to IntelliJ 2018.2 engine, started in #7499 (which didn't help).

BTW, the master build now uses this version and all that problems can be seen here: https://teamcity.jetbrains.com/viewType.html?buildTypeId=OpenSourceProjects_Druid_Inspections

@leventov leventov added Development Blocker Area - Dev For items related to the project itself, like dev docs and checklists, but not CI labels Apr 24, 2019
@leventov leventov requested a review from egor-ryashin April 24, 2019 14:51
Copy link
Copy Markdown
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

The inspection changes look fine, so I just have questions about the two new copied-in files.

Comment thread .idea/misc.xml
</value>
</option>
</component>
<component name="ProjectResources">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Due to the fact that these files appear to be copied from elsewhere, I have a some questions about them:

  • Why do they need to be added, rather than referenced somehow without copying? (Would make a good comment here)
  • Will there be a need to update them in the future, and if so what would trigger that? Or are these files not expected to update regularly?
  • I don't see a license attached to assembly-2.0.0.xsd -- meaning we need to add a comment describing the legal basis for being able to include this file. Ideally this comment takes the form of a statement of what license the file is being used under, and why we believe it falls under that license. Even separately from the legal reason for including a comment, it would be good just to document where the file came from.
  • I do see a license and source URL in a comment at the top of svg11.dtd, which is good.

Copy link
Copy Markdown
Member Author

@leventov leventov Apr 24, 2019

Choose a reason for hiding this comment

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

Why do they need to be added, rather than referenced somehow without copying?

IntelliJ verifies XML documents by the schema. XML documents usually reference those schemas as URLs:

<assembly xmlns="http://maven.apache.org/ASSEMBLY/2.0.0"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://maven.apache.org/ASSEMBLY/2.0.0 http://maven.apache.org/xsd/assembly-2.0.0.xsd">

But IntelliJ doesn't automatically go to the internet to download the resource. It needs to know what schema corresponds to what URL, statically.

(Would make a good comment here)

Unfortunately, IntelliJ has a bad habit of purging comments from these settings XMLs, so adding comments is not productive: https://youtrack.jetbrains.com/issue/IDEA-211087.

Will there be a need to update them in the future, and if so what would trigger that? Or are these files not expected to update regularly?

No, these schema documents are expected to be immutable forever. If it will be updated, it will be http://maven.apache.org/xsd/assembly-2.0.1.xsd, or whatever another version.

I don't see a license attached to assembly-2.0.0.xsd -- meaning we need to add a comment describing the legal basis for being able to include this file. Ideally this comment takes the form of a statement of what license the file is being used under, and why we believe it falls under that license. Even separately from the legal reason for including a comment, it would be good just to document where the file came from.

This document is created and managed by Apache Maven, so it must be under Apache license. I don't feel confident to edit the file itself to add a license header so that it actually diverges from the contents of http://maven.apache.org/xsd/assembly-2.0.0.xsd.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unfortunately, IntelliJ has a bad habit of purging comments from these settings XMLs, so adding comments is not productive: https://youtrack.jetbrains.com/issue/IDEA-211087.

I don't think this comment is essential but it would be helpful. Maybe a README in the .idea directory?

This document is created and managed by Apache Maven, so it must be under Apache license. I don't feel confident to edit the file itself to add a license header so that it actually diverges from the contents of http://maven.apache.org/xsd/assembly-2.0.0.xsd.

My understanding is that we must note this somewhere as a matter of legal hygiene. I think it's fine to edit the file to add a comment at the top with source and licensing info, something like:

<!-- Other than this comment, this file is identical to
     https://maven.apache.org/xsd/assembly-2.0.0.xsd, which, as part of
     Apache Maven, is licensed under the Apache 2.0 license.
  -->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the comment would be good. The files in .idea are not bundled as part of the source or binary assembly, so the licensing info for this wouldn't belong in LICENSE; the header seems like the best place.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think this comment is essential but it would be helpful. Maybe a README in the .idea directory?

It would be so far from the site that it won't serve the purpose: people don't have the means to know that there is a comment about something in a completely different file. But I've created #7549 about this. Please vote for https://youtrack.jetbrains.com/issue/IDEA-211087 so that hopefully it's fixed faster than never.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've added

<!-- Other than this comment, this file is identical to
     https://maven.apache.org/xsd/assembly-2.0.0.xsd, which, as part of
     Apache Maven, is licensed under the Apache 2.0 license.
  -->

to the file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

However, added .idea/README.md with comments anyway.

Copy link
Copy Markdown
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Dev For items related to the project itself, like dev docs and checklists, but not CI Development Blocker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants