Revive Project and upstream changes from fork

[svrooij]

This project doesn't seem to get any updates anymore.

Recently I tried to restart development for it in my fork.

It's configured for automatic releases and it does run all the tests (but because of some failing tests, we are still continuing when we get an error this WILL change once we fixed all the tests).

The new library also got a place on NPM, so everybody can just install it an use it whenever they want.

I'm willing to merge it back upstream and rename back if we could arrange that I could manage the repo and the packages, because no one seems to be doing that at the moment.

[mk-pmb]

Thanks for notifying us. There are indeed several forks already, that do publish to npm. Nonetheless, you can still install from this repo (see #115), even using npm with Github dependency syntax.

[svrooij]

We also did some fixing on the code, the tests and the release strategy (fully automated with GitHub actions). I was just offering to at least merge it to the nodeftpd repo (with no people in the organization). So we could have a wider audience then just our fork.

I'm fine with keeping the package in my npm namespace or to create a new npm namespace. If the current package name ftpd is not under your control my suggestion is creating a organization with that name and publishing a package that way.

I'm not a big fan of GitHub installs, but that is a personal opinion.

[sstur]

Hey @svrooij, thanks for reaching out!

This project doesn't seem to get any updates anymore.
Recently I tried to restart development [...]

This is really awesome 🎉
I'm also interested in breathing some life back into this project, so I'd be keen to pull your changes upstream and get you added as a contributer here.

Let me take a look at your repo and the latest work you've done and add you here. Then let's maybe have a conversation about what our goals are with this and see where that might overlap so we can collaborate.

Looking forward to some future for this project after a long time!

[sstur]

Hey, @mk-pmb, you've contributed to this project quite a bit in the past, and you seem active at replying to stuff. If you're interested, I'll also add you as a collaborator to help us revive this project.

[svrooij]

Should I create a PR with the wanted situation?
Currently the tests are only run, to see how they are doing, the result is skipped because of some failing tests. The semantic release runs before it.

I also setup some eslint style checking and did an automated fix all with eslint. This makes the changes pretty big.

I also have setup coveralls to check code coverage.

Actions needed to merge the current fork to nodeftpd:

• Setup an NPM_TOKEN in the repository secrets with publish permissions (owner has to do this)
• Go to coveralls and check the box for nodeftpd.
• Set failing tests to skip, and move the tests up in the chain (no more release without tests)
• Change readme back (it now has a message about some tests failing and the namespaced version), also the links have to be changed back to the original
• Push the changes to a new branch on nodeftpd (so it wil check everything but not do a release, only releases from master / beta / next where the last two will produce a pre-release version.)

Starting now each commit has to start with fix: (for patch release) or feat: for a minor release.

[sstur]

Most of this sounds good. I think there's some room for discussion around which branch we auto-release from and what npm package we will release to (I'll register @nodeftpd/nodeftpd as the official package, but open to discuss). I'm hesitant to release directly from master but open to being convinced.

Also, let's discuss the ESLint auto-fixing. Not sure if that should be part of the CI, but maybe I'm misunderstanding that part.

Let's open an issue for each one of these, and create a label of some kind so I can filter it out from all the other issue noise. Maybe mention me on the issue.

About the commit message syntax, I'm totally cool with the semantic style, in fact we can enforce that with a semantic.yml file in the .github directory if I understand correctly.

[mk-pmb]

As long as we release rarely enough that we can afford having a human in the loop, let's not automatically publish to npm. Storing publish tokens in the cloud opens several new attack vectors that I don't currently see justified.

People who really need the new releases asap, can put the Github repo as their dependency; all others can probably wait a few days for normal releases. Only security fixes could be more urgent, but having to issue them means there was a problem in our procedures, which to me means human attention to detail and circumstance is even more important with security fixes.

[svrooij]

Also, let's discuss the ESLint auto-fixing. Not sure if that should be part of the CI, but maybe I'm misunderstanding that part.

At the moment it only forces the code to comply to the eslint config. But I did a eslint --fix on my local machine and push back the results. That makes a lot of files being changed without changing functionallity. svrooij@77db42d

[mk-pmb]

Also, let's discuss the ESLint auto-fixing. Not sure if that should be part of the CI,

As far as I understood, and was just verified, it's verify-only. The CI should never auto-modify files that are meant to convey human intent.

That makes a lot of files being changed without changing functionallity.

The latter part is always what we hope, at least. :-D

[svrooij]

So to summarize @mk-pmb you would rather have it setup that the actual publish to npm isn’t automated? I’m a big fan of automating tasks that are always the same and don’t need my involvement.

Then I’ll set it up to do Eslint verify and then run the tests (I’ll skip the failing tests for now).

[sstur]

Yea, so to jump in here a bit late, I kinda also prefer to make master branch the development branch and setup automated deployment on a different branch (e.g. release branch).

We would manually do a branch cut whenever we choose, simply pushing the content from master to release. (or automate the branch cut once per week/month/whatever)

Note, the deployment script that runs on the release branch should not actually push new changes to that branch, (version bump, etc) rather it should create a new tag for each release.

Hopefully that makes sense.

[sstur]

rather have it setup that the actual publish to npm isn’t automated?

To more directly answer the question, I think the process should be automated, but I hesitate to make the trigger automated, especially if the proposed trigger is every push to master.

I’m a big fan of automating tasks that are always the same and don’t need my involvement.

Yes, I 100% agree with this statement.

[mk-pmb]

How about we make a stable branch with an npm script npm run ci-publish that goes through all the verification steps and even creates an npm tarball (npm pack) as a GitHub actions artifact, to ensure that everything except the decision to publish is automated. That way everyone on the team can help fix any obstacles that may occurr, people can even test the npm tarball that would be released. Except the GitHub actions don't have an npm token.

This way the maintainers can have trust that if they deem the current stable branch worthy of publishing, all they have to do is run npm run ci-publish in a shell that does have a proper npm token.