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

Add unit tests and CI #21

Closed
jw3 opened this issue Oct 11, 2021 · 5 comments · Fixed by #33
Closed

Add unit tests and CI #21

jw3 opened this issue Oct 11, 2021 · 5 comments · Fixed by #33
Milestone

Comments

@jw3
Copy link
Member

jw3 commented Oct 11, 2021

The unit tests and workflow configuration were removed in jw3/example-daffodil-vscode@92d5c7d to simplify licensing for the move.

These need added back, while this time using schema and test data that are license compatible with this repo.

@stevedlawrence
Copy link
Member

When this is added, I suggest you base it off of the Daffodil file. Specifically, ignoring dependabot branches, test matrices (if needed), Apahce Rat check, Single commit PR, etc.

@shanedell
Copy link
Contributor

shanedell commented Oct 14, 2021

@stevedlawrence I was working on this one and was trying to follow the apache/daffodil/.github/workflows/main.yml for how to setup the CI.

However when it comes to the Apache Rat Check, would we want to get that setup as separate PR? I think I could easily get that stuff added in but I wanted to see how thought best to approach this? I did notice the comment on #26 about this so wanted to make sure to verify best solution.

Do you want the CI for this project to be the same as apache/daffodil where it is doing multiple things in the one CI? As I was just going to make a .github/workflows/tests.yml for running the unit tests and more or so making all CI items being its own separate thing, but again want to make sure I do what is proper in this case.

@stevedlawrence
Copy link
Member

I'm fine with Rat being a separate PR.

In Daffodil we did have a couple separate yml files, but we eventually combined into a single file. We found it reduced duplication and made it a bit easier to manage and visually everything that is checked in a PR. So I'd lean towards that. But this is a different project so if that doesn't make sense I'm open to alternatives. But the more similar things are, the easier it is for people to switch become familiar with both projects.

@shanedell
Copy link
Contributor

That makes sense, do you think it would be best for us to keep them separate for the time being then once we have the pieces we want in the CI we can merge them together?

@stevedlawrence
Copy link
Member

I have no strong preference. It'll probably be easier to make suggestions once we see the actual checks (it's not totally clear to me what is involved with CI for this vscode project). So I suggest do what you feel is right, and once we have something concrete to look at we can determine if there's a better way to organize things.

@shanedell shanedell mentioned this issue Oct 15, 2021
shanedell referenced this issue in shanedell/daffodil-vscode Oct 27, 2021
stevedlawrence pushed a commit that referenced this issue Oct 27, 2021
stevedlawrence pushed a commit to stevedlawrence/daffodil-vscode that referenced this issue Nov 17, 2021
…fodil-debug (apache#21)

update the downloading of the debugger files into the home directory to
work on windows

delete some bad white space and comments

update location of the downloaded files to go to tmp folder using os.tmpdir

update location of downloaded files

fix data directory name
`

get rid of information message
@shanedell shanedell added this to the 1.0.0 milestone Apr 4, 2022
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 a pull request may close this issue.

3 participants