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

Feature/trx reporter #17

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

TamaMcGlinn
Copy link
Contributor

Continuing from PR 14; since the combined reporter is deleted entirely it seems fitting to change the branch name to just feature/trx_reporter.

Also, because of PR 3 adding Set_File, we can leave that logic out entirely and don't need to hardcode the filename.

There is still a problem that I used Ada.Containers.Vectors and Ada.Calendar in the implementation, which makes it impossible to use in restricted environments such as zfp.

@TamaMcGlinn
Copy link
Contributor Author

How strict is AdaCore's 80 character line limit rule? I have a url in the code which is a lot less useful if broken up. That's the only line over 80 now.

@fedor-rybin
Copy link
Contributor

How strict is AdaCore's 80 character line limit rule? I have a url in the code which is a lot less useful if broken up. That's the only line over 80 now.

Usually for the cases like these where it makes sense to keep it as one line, it should be wrapped into pragma Style_Checks:

pragma Style_Checks ("M200"); -- Allow long lines
<really long line than should be kept like that>
pragma Style_Checks ("M79");

@fedor-rybin
Copy link
Contributor

As for Ada.Containers.Vectors, Ada.Calendar, Ada.Text_IO, and Ada.String.Unbounded, it looks like they are used quite extensively in the reporter. So my suspicion is that it would be indeed way easier to make this reporter available for the full runtime only.

Tama McGlinn and others added 5 commits March 9, 2021 21:43
…ild system can display graphically

removed finalization from aunit reporter

fixed Report signature; Result is no longer in out

Added copyright notice to trx reporter

break all lines over 80 characters, except the url

use Engine.File rather than hardcoded testresult.trx
It was decided in TB02-012 that, even though out is now unused,
we should keep it to remain compatible with other client's custom
reporters.
I decided to just add a full_runtime directory;
any other reporters that people want to add can be added
to this directory
@TamaMcGlinn TamaMcGlinn force-pushed the feature/trx_reporter branch from 83b582f to 701ce8a Compare March 9, 2021 21:23
@TamaMcGlinn
Copy link
Contributor Author

Sorry this took so long; it was just not that high on my priority list. I believe I have fixed all outstanding issues on this now;

  • line length
  • only include for full_runtime
  • revert to in out for the Report procedure following 31fe97c

I also saw a typo in a Makefile that was preventing me from doing Make clean - seems minor enough to just throw in even though unrelated.

@fedor-rybin Could you re-review? If other ACT customers had custom reporters which also need the full runtime, it should be really easy now to add them, so maybe you could ask if they want to open-source those.

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