Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

CB-14224 Default.rd.xml header fixes #284

Merged
merged 2 commits into from
Aug 1, 2018
Merged

Conversation

brodycj
Copy link

@brodycj brodycj commented Jul 27, 2018

Original issue

https://issues.apache.org/jira/browse/CB-14224

Platforms affected

Windows

What does this PR do?

template/Properties/Default.rd.xml header fixes:

Rationale: I think it is desired to add license headers whenever possible to satisfy Apache RAT license auditing tool. While I think it should be possible to configure the auditing tool to skip this file I would rather follow the practice of adding the license text whenever possible.

NOTE: I also proposed updates to LICENSE text in microsoft/Windows-universal-samples#949 (not included in this PR).

What testing has been done on this change?

I verified that coho audit-license-headers -r windows with these changes does not show the missing license header any more.

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • [ ] Added automated test coverage as appropriate for this change.

@brodycj brodycj changed the title CB-14224 add license text & GitHub link to template/Properties/Default.rd.xml - PROPOSAL WIP [WIP] CB-14224 add license text & GitHub link to template/Properties/Default.rd.xml - proposal (for discussion) Jul 27, 2018
@@ -13,6 +13,28 @@

Using the Namespace directive to apply reflection policy to all the types in a particular namespace
<Namespace Name="DataClasses.ViewModels" Seralize="All" />

Copy link
Member

Choose a reason for hiding this comment

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

Is this missing the start of the xml comment block?

@brodycj
Copy link
Author

brodycj commented Jul 27, 2018 via email

@shazron
Copy link
Member

shazron commented Jul 27, 2018

Ah I see now, when viewing the whole file, i see the start of the comment block, my mistake, I thought you added the MIT license in there without it being a proper comment block.

@brodycj brodycj changed the title [WIP] CB-14224 add license text & GitHub link to template/Properties/Default.rd.xml - proposal (for discussion) [WIP] CB-14224 Default.rd.xml header fixes Aug 1, 2018
@brodycj brodycj changed the title [WIP] CB-14224 Default.rd.xml header fixes CB-14224 Default.rd.xml header fixes Aug 1, 2018
@brodycj brodycj self-assigned this Aug 1, 2018
@brodycj
Copy link
Author

brodycj commented Aug 1, 2018

As this change is already part of 6.0.x (GH-285) it should now be in master. Title and description have been updated.

@brodycj brodycj merged commit 6572801 into apache:master Aug 1, 2018
@brodycj brodycj deleted the CB-14224 branch August 1, 2018 03:46
janpio pushed a commit that referenced this pull request Sep 2, 2018
…ies/Default.rd.xml (#286)

### Original issue

<https://issues.apache.org/jira/browse/CB-14225>

### Platforms affected

Windows

### What does this PR do?

- Fix sample Namespace Serialize attribute in `template/Properties/Default.rd.xml`, according to `Default.rd.xml` in C# UWP project generated by Visual Studio 2017

I also raised <microsoft/Windows-universal-samples#950> to apply a similar fix there.

Also related:
- #284 - fix license text & add link to `template/Properties/Default.rd.xml` (as reported in <https://issues.apache.org/jira/browse/CB-14224>)
- <microsoft/Windows-universal-samples#949> - fix license txt (related to but not part of #284)

### What testing has been done on this change?

- Visual inspection
- check diff

### Checklist

- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
- [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
- ~~Added automated test coverage as appropriate for this change.~~
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants