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 file: protocol and better Windows shims, cleaning up cruft #30

Merged
merged 12 commits into from
May 16, 2022

Conversation

pygy
Copy link
Member

@pygy pygy commented Jan 15, 2021

This is another take on #28, plus some cleanup.

We add the file:, a dev dependency on cli-shims to get the exact same node_modules/.bin shims as NPM.

I also removed the 'old style count' from the log, and the corresponding infrastructure.

I still have to fix the release instructions and the change log.

How Has This Been Tested?

I tried it the best I could from macOS (by using the shims there too, it runs the bin/sh one), but this needs to be tested in Windows before it is merged.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read all applicable contributing documents.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the change log (if applicable).

Copy link
Member

@dead-claudia dead-claudia left a comment

Choose a reason for hiding this comment

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

Add Windows to the testing matrix, to prevent this from happening in the future. https://docs.travis-ci.com/user/reference/windows

Once you do that (and assuming it passes there), it's got my stamp of approval.

@dead-claudia
Copy link
Member

@pygy You have a status update on this? We got another person reporting the same issue: #32

@dead-claudia
Copy link
Member

@pygy Status update? #35 is reporting this very thing.

@pygy
Copy link
Member Author

pygy commented Jun 7, 2021

Oh hai Claudia, I'll take a look hopefully this evening

@pygy pygy force-pushed the 2021-jan-01 branch 14 times, most recently from 6f6099f to f0795cf Compare June 12, 2021 18:18
Copy link
Member

@dead-claudia dead-claudia left a comment

Choose a reason for hiding this comment

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

In addition to the couple nits, please also rip out .travis.yml (Edit: and remove/replace other Travis-related stuff as applicable like the badge in the README) if you're going to switch to GitHub Actions for testing.

I wasn't expecting the more invasive changes, but I'm okay with the rest modulo the above.

.travis.yml Outdated Show resolved Hide resolved
Comment on lines +1011 to +1013
var shortDelay = 10
var middleDelay = 50
var longDelay = 200
Copy link
Member

Choose a reason for hiding this comment

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

🎉

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

pygy commented Jun 13, 2021

Yes, travis.org is about to go poof by June 16th apparently, they suggest one migrates to travis.com which, right off the bat tries by default to get write permission to all of one's repos (it can be worked around easily, but it doesn't inspire confidence).

I had read that the new owner was shady, and this confirmed my impression, hence the move to GH actions.

I'll take your suggestions into account... This is in a WIP state obviously, I'm making progress in a dedicated branch over at https://github.com/pygy/ospec/tree/actions-tests to avoid spamming the gitter log.

@dead-claudia
Copy link
Member

dead-claudia commented Jun 14, 2021

I had read that the new owner was shady, and this confirmed my impression, hence the move to GH actions.

@pygy On that note, follow up across all our repos once you get this one in shape.

@pygy
Copy link
Member Author

pygy commented Jun 14, 2021

@isiahmeadows We use the previous stable release to run the current test suite, but this won't work for CI on Windows (because the ESM tests won't work there with the stable version), so I propose we release, on a distinct NPM channel, one version that uses itself as a test harness, then release a stable release that relies on the aforementioned (and bump the devDep immediately after publishing so that future versions once again can rely on the previous stable release).

Edit, actually, no need to do a custom release, I can have v4.1.2 depend on a git hash rather than the NPM package

On that note, follow up across all our repos once you get this one in shape.

Gotcha

@dead-claudia
Copy link
Member

dead-claudia commented Jun 14, 2021 via email

package.json Outdated Show resolved Hide resolved
@pygy pygy marked this pull request as ready for review May 16, 2022 16:17
@pygy pygy merged commit df843d6 into MithrilJS:master May 16, 2022
@pygy pygy deleted the 2021-jan-01 branch May 16, 2022 22:47
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.

None yet

2 participants