feat: Generate Ignore.xml #18
Conversation
workgroupengineering
force-pushed
the
features/Generate_Ingnore.xml
branch
from
9468ae1
to
c3d11aa
Compare
workgroupengineering
force-pushed
the
features/Generate_Ingnore.xml
branch
from
e356d6a
to
e67d5c2
Compare
src/Uno.PackageDiff/GitHubClient.cs
Outdated
|
|
||
| var result = await wc.UploadStringTaskAsync(contentUri, "POST", document); | ||
|
|
||
| return JsonConvert.DeserializeObject(result); |
There was a problem hiding this comment.
dynamic seem like a worse idea, than just returning the string
but, if you arent using the result pass debugging stage
it is better to just let this be a Task of nothing
|
|
||
| if(types.Count + properties.Count + events.Count + methods.Count + fields.Count > 0) | ||
| { | ||
| sw.WriteLine("🚨🚨**detected some breaking changes** if changes are wanted, add following line at end of `PackageDiffIgnore.xml`.🚨🚨"); |
There was a problem hiding this comment.
the sections can be long
you might want to wrap them in <details> syntax:
like so
<details>
<summary>like so</summary>
```xml
...// note: you need to leave a empty line between <summary> and start of ```
```
</details>
|
|
||
| namespace Uno.PackageDiff | ||
| { | ||
| internal interface IDiffWriter : IDisposable |
There was a problem hiding this comment.
this contract looks highly tied to xml syntax (WriteXyzStart, WriteXyzEnd), and md formatting (XyzStart that takes in an int).
it should instead accept the array of missing types, and (containing types + their missing members: properties,fields,method and events)
or anything more context-relevant
| }; | ||
|
|
||
| p.Parse(args); | ||
|
|
||
| IDiffWriter writer = null; | ||
| if(string.IsNullOrEmpty(outputType)) |
There was a problem hiding this comment.
the if-else can be extracted to a separate method:
using (var writer = BuildWriter(outtype, /* github params... */))| foreach(var type in types) | ||
| { | ||
| if(string.Equals("md", type, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| composite.Add(new Writers.MDDiffWriter(outputFile)); | ||
| } | ||
| else if(string.Equals("diff", type, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| composite.Add(new Writers.XmlIngnoreWriter(outputFile)); | ||
| } | ||
| else if(string.Equals("github", type, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| composite.Add(new Writers.GitHubWriter(githubPAT, sourceRepository, githubPRid)); | ||
| } | ||
| } | ||
| if(composite.Count == 0) | ||
| { | ||
| Console.WriteLine($"Error : Invalid outtype {outputType}."); | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
a combination of valid and invalid outtype will slip through here... ex: "outtype=diff,asd"
you should add a final else: else { Console.WriteLine($"Error: Invalid outtype: {type}"); }
and change the final if to composite.Count != types.Length
|
|
||
| namespace Uno.PackageDiff.Writers | ||
| { | ||
| internal class XmlIngnoreWriter : IDiffWriter |
There was a problem hiding this comment.
| internal class XmlIngnoreWriter : IDiffWriter | |
| internal class XmlIgnoreWriter : IDiffWriter |
and, the same in commit+pr title too
workgroupengineering
changed the title
|
Replaced by #20 |
GitHub Issue (If applicable): #
PR Type
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior?
Allowed to generate Ingnore.xml
PR Checklist
Please check if your PR fulfills the following requirements:
Other information
Internal Issue (If applicable):
Part of unoplatform/uno#10885