Skip to content

Embed coverage-04.dtd into binary#11

Merged
twittemb merged 3 commits intotwittemb:mainfrom
Galarius:feature/offline-coverage-04-dtd
Jun 1, 2023
Merged

Embed coverage-04.dtd into binary#11
twittemb merged 3 commits intotwittemb:mainfrom
Galarius:feature/offline-coverage-04-dtd

Conversation

@Galarius
Copy link
Copy Markdown
Contributor

@Galarius Galarius commented May 20, 2023

  • Embed coverage-04.dtd into binary

    The main purpose of embedding the coverage-04.dtd into xcc
    is to allow xcc to work in certain deployment environments where there may be restrictions on Internet access.

  • Fix swift build warning

    warning: '--build-path' option is deprecated; use '--scratch-path' instead

  • Add smoke test

The description has been edited to reflect the latest changes.

@Galarius
Copy link
Copy Markdown
Contributor Author

@twittemb, another approach is to embed coverage-04.dtd in the binary. Would it make sense?

@twittemb
Copy link
Copy Markdown
Owner

@twittemb, another approach is to embed coverage-04.dtd in the binary. Would it make sense?

Hi, thanks for the PR.
I can see this mechanism:

  • by default the DTD is embedded in the binary
  • we offer the possibility to provide a URL (local or remote) as a new parameter if the user wants to customise it

does it make sense ?

@Galarius
Copy link
Copy Markdown
Contributor Author

If it is fine to embed coverage-04.dtd in the binary, I honestly don't see any benefit in providing a custom DTD URL. The main goal is to be able to run xcc in the network isolated environment.

A number of keywords are hardcoded, so providing a DTD file with different content will likely break the converter. Moreover, introducing a new parameter requires some refactoring to be done, because both supported converters conform to the same interface, but only cobertura-xml will need this parameter.

@twittemb
Copy link
Copy Markdown
Owner

twittemb commented May 24, 2023

If it is fine to embed coverage-04.dtd in the binary, I honestly don't see any benefit in providing a custom DTD URL. The main goal is to be able to run xcc in the network isolated environment.

A number of keywords are hardcoded, so providing a DTD file with different content will likely break the converter. Moreover, introducing a new parameter requires some refactoring to be done, because both supported converters conform to the same interface, but only cobertura-xml will need this parameter.

Good analysis. I think it is fair to embed it in the binary and that's it.

Galarius added 3 commits May 25, 2023 20:00
> warning: '--build-path' option is deprecated; use '--scratch-path' instead
The main purpose of embedding the [coverage-04.dtd](http://cobertura.sourceforge.net/xml/coverage-04.dtd) into `xcc`
is to allow `xcc` to work in certain deployment environments where there may be restrictions on Internet access.
@Galarius Galarius force-pushed the feature/offline-coverage-04-dtd branch from 6a35f58 to c6a9b7c Compare May 26, 2023 07:17
@Galarius Galarius changed the title Add UserDefaults support for DTD URL configuration Embed coverage-04.dtd into binary May 26, 2023
@twittemb twittemb merged commit 35b9df9 into twittemb:main Jun 1, 2023
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