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 code and tests for checking DOM Node types #4

Merged
merged 15 commits into from
Sep 15, 2023

Conversation

cjbarth
Copy link
Collaborator

@cjbarth cjbarth commented Sep 10, 2023

No description provided.

@cjbarth cjbarth marked this pull request as ready for review September 14, 2023 15:14
Copy link
Member

@karfau karfau left a comment

Choose a reason for hiding this comment

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

Great work, added some comments.
Really like the simple, straight forward contribution guide.

README.md Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function isNodeLike(value: any): value is Node {
Copy link
Member

Choose a reason for hiding this comment

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

The definition of Node is coming from the DOM lib of typescript, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct. This would be the same definition that would be in use if they used TypeScript in a browser.

src/index.ts Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
test/index.spec.ts Outdated Show resolved Hide resolved
.github/dependabot.yml Outdated Show resolved Hide resolved
@@ -0,0 +1,7 @@
# Changelog
Copy link
Member

Choose a reason for hiding this comment

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

One more question, is the release workflow requiring you to already put the updated changelog into the branch?
And did you manage to already publish it somewhere, which triggered the creation of this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried running my tooling to generate the file, but it generated an empty file because there is no tag to look at. After the first release, it will automatically build a changelog from all the PRs. Look at this as a placeholder. If you don't want it there now (because it is empty), that's fine; I'll remove it.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@karfau
Copy link
Member

karfau commented Sep 15, 2023

Not sure why this PR is not using your workflow files, but I guess it is because you created the PR from a fork.
The checks tab says:
"Workflow runs completed with no jobs"

Would be curious to see them running before merging.
Could you push the branch to this repo and see if that works?
(I think that requires a new PR)

@codecov
Copy link

codecov bot commented Sep 15, 2023

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@cjbarth cjbarth merged commit edf12a7 into xmldom:main Sep 15, 2023
9 checks passed
@cjbarth cjbarth added the enhancement New feature or request label Sep 15, 2023
@cjbarth cjbarth changed the title migrate all the code from xpath project Add code and tests for checking DOM Node types Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants