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 fastlane-env-reminder GitHub Action #2

Merged
merged 13 commits into from
Sep 13, 2019

Conversation

mollyIV
Copy link
Member

@mollyIV mollyIV commented Sep 4, 2019

Motivation and Context

Some time ago GitHub has announced GitHub Actions. Recently a fastlane organisation got an access to Actions Beta. It is a good time to start playing with the new feature from GitHub.

It is the first step of bot's code migration to GitHub Actions based on this proposal.

This PR is introducing an action for adding a comment to include fastlane env in an issue description.

closes #1.
reference to the epic: #3.

Description

I followed the instructions from Creating a JavaScript Action guidelines and Getting Started: GitHub context pull request.

I was looking at the examples from GitHub Actions repository, i.e. first-interaction.

Please note that this is just an action implementation. It does not take any effect itself. We still need to create the workflow, which will be using the add-fastlane-env-comment action. I will create dedicated issue for that once we merge this PR.

There's a lot of changes in this PR because I needed to commit production dependencies. To make a review easier, I did split the commits:

  • Add GitHub Action for adding a comment to include fastlane env in an issue description 👈 actual code changes
  • Add production dependencies 👈 adding production dependencies

We proposed a solution how to avoid such a big PR in the comments below.

Testing

To test the changes, I implemented the unit tests. To run them, execute npm test (followed by npm install to install the package dependencies and npm build to build the package).

image

🙇 🙇

@janpio
Copy link
Member

janpio commented Sep 4, 2019

Is there no way to do the "commit bundled dependencies" step automatically in another branch so the main branch is clean? (Maybe there is a Github Action for that... ;) )

@mollyIV
Copy link
Member Author

mollyIV commented Sep 4, 2019

Is there no way to do the "commit bundled dependencies" step automatically in another branch so the main branch is clean? (Maybe there is a Github Action for that... ;) )

That would be lovely @janpio ❤️

It is hard to read the PR changes due to node_modules dependencies. To make it easier, I split the commits. I had a look at GitHub Actions repository and it looks for javascript projects they were also creating big PRs.

An alternative was to zip the dependencies, like they did in first-interaction action. However I can imagine that updating the dependencies would be harder than updating them in package.json.

@mollyIV
Copy link
Member Author

mollyIV commented Sep 4, 2019

👋 @janpio,

added node_modules to .gitattributes file, so the generated files are not rendered by default. I hope it will make a review a bit easier.

image

However please have a look at the next comment below 👇

@mollyIV
Copy link
Member Author

mollyIV commented Sep 4, 2019

I did read a Dev Workflow documentation again and I believe it is a way to go:

The workflow below describes one possible workflow with a branching strategy. Others exist.

Key Point: the branch that users reference in their workflow files should reference an action that has only the production dependencies.

The workflow below describes a strategy where you code in master (with node_modules ignored) with a v1 branch users reference and contains the product references. Actions are self contained referenced on the github graph of repos.

In the future, GitHub will provide tooling and an action to automate it. According to Publish a v1-release Action:

NOTE: We will provide tooling and an action to automate this soon.

So basically we would have master branch (with node_modules ignored) and latest branch.
The usage in a .github/workflows/workflow-a.yml file would look like 👇

-uses: fastlane/github-action/fastlane-env-reminder@latest

The benefits of the described workflow:

  • easy to review Pull Requests
  • small size and fast checkout of master branch (with node_modules ignored)

We could also consider tagging the releases, however we will have multiple actions in the same repo so it might be an issue. Tagging the releases should be a go for a single, public action though.

What do you think? 🤔

@janpio
Copy link
Member

janpio commented Sep 5, 2019

Ja, that sounds so much more sane.

Is this doable manually for now?

We could also consider tagging the releases, however we will have multiple actions in the same repo so it might be an issue. Tagging the releases should be a go for a single, public action though.

You can always prefix the tag and release with the project name to be able to use this functionality properly. As we want to have multiple actions in here, we might have to do that anyway (unless we want to release them all at the same time).

@mollyIV
Copy link
Member Author

mollyIV commented Sep 5, 2019

Is this doable manually for now?

Totally 👍We can add production dependencies on latest branch manually without any issues.

You can always prefix the tag and release with the project name to be able to use this functionality properly.

Sure 👍 Initially, the idea is to release all the actions at the same time. So we will be creating a PR to the latest branch with adding only production dependencies for all the actions. Obviously the flow is gonna be different for "public" actions.

Copy link
Member

@janpio janpio left a comment

Choose a reason for hiding this comment

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

Generally looks good!

@mollyIV mollyIV merged commit 737ac37 into fastlane:master Sep 13, 2019
@mollyIV mollyIV deleted the fastlane-env-reminder branch September 13, 2019 07:33
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.

Add GitHub Action for adding a comment to include fastlane env in an issue description
2 participants