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

Road ahead #48

Open
dbartholomae opened this issue Jan 12, 2023 · 7 comments
Open

Road ahead #48

dbartholomae opened this issue Jan 12, 2023 · 7 comments

Comments

@dbartholomae
Copy link
Contributor

Hi! After the first PR, I would like to talk a bit about the road ahead before taking this on. On the meta level I would like to understand what I can do to get PRs merged faster, and, on a content level, align on a roadmap.

For the roadmap, I currently see these steps:

  1. Automatically publish new versions of this package to npm (this requires hands-on support as I don't have the key to publish to npm and therefore can't set this up alone)
  2. Update the actions to the new version, or even add something that automatically updates them when this package changes
  3. Add the ideas from Make alert messages in issue more helpful #44 and related tests.

I'm happy about feedback on the roadmap, but would also love to understand what I can do to speed up the merge process and discussion. I could e.g. offer to have a synchronous 30 minute session when a PR is done with everyone who is needed to approve the PR to discuss what needs to be done. But I'm also open to other suggestions.

What do you think?

@thc202
Copy link
Member

thc202 commented Jan 12, 2023

  1. If with automatically you mean whenever we decide that should be released, sounds good to me. This can be done with a GitHub action with workflow dispatch I guess.
  2. dependabot should do that when a new version is released.
  3. Sounds good.

What I think would make PRs be merged faster is tests that ensure that the existing functionality (and new being added) is not being broken. Since there were no tests at all things need to be looked more thoroughly, also worth noting that we work on ZAP on our free time (most of the time at least).

Note that the long term plan is to provide an action that makes use of Automation Framework.

@dbartholomae
Copy link
Contributor Author

Thanks! Also to be clear, I don't want to criticize or assume any SLAs for open-source software. I'm an open-source maintainer myself (though only for smaller projects) and als owork on this project in my evenings, so I fully understand that sometimes there is no time to look at things. I'm mainly trying to understand what I can do so that PRs need to be looked at only once or twice and then decided upon.

Testing is a good starting point - with the tests and the GitHub action in place now, some changes (mainly from step 3) should be really easy to review. For step 1, this won't be as easy, unfortunately, and will also require more support than just merging a PR.

@psiinon
Copy link
Member

psiinon commented Jan 13, 2023

@dbartholomae I think one of the key things we need is someone to take ownership of the actions.
And I'd be delighted for that to be you!
As @thc202 said another key thing is tests - right now we dont have any confidence things wont brerak without manual testing .. and thats time consuming and painful.

I'd also like to see some more docs explaining how things fit together and how to test them (where any manual testing is needed).

Would a video call be a good option? We can organise that via email / twitter if you like - I think you know my accounts?

@dbartholomae
Copy link
Contributor Author

Quick summary from our call:

  1. We do not consider this package itself a part of Zaproxy's public interface and therefore are less cautious about breaking changes here. The actual public interface is on the level of the actions.
  2. I'll add a setup to this repo for automatically publishing changes to npm. The PR will include pointers on how to add the NPM publish token to the secrets of this repo.
  3. Next, I'll take one of the actions (e.g. the baseline action) as a starting point and make it work with the current version of this repo. I don't expect any problems here, but since the code in this repo when I started was already out-of-sync with the npm package, there might something unexpected happen.
  4. Once the action is working, I'll add a manual release process to the action repo.
  5. Next, I'll roll out the same process to the other actions.
  6. Last but not least, I'll add a feature for better GitHub issues to this repo, which will then walk through the full release process.

I might also make other small improvements to the existing codebase wherever I touch it, but only, if it makes reviewing the code easier.

@thc202
Copy link
Member

thc202 commented Jan 18, 2023

  1. They were not the same because new features were being added, Add artifact name as argument but enforce a positional default #15 (see also artifactName: Fix multiple issues being created when passed (WIP) #21, which IMO should be addressed before release).
  2. We already have a manual release process.

@kingthorin
Copy link
Member

I guess for no.4 he means "semi-automated", in that it's automated but we'll have to kick it off 'manually'.... 🤷 I could be wrong though.

@dbartholomae
Copy link
Contributor Author

Thanks! Yes, better wording would be "I'll automate more around the manual release process" :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants