IITC-CE / ingress-intel-total-conversion

intel.ingress.com total conversion user script with some new features. Should allow easier extension of the intel map.
https://iitc.app
ISC License
290 stars 109 forks source link

Idea: development `test` branch #56

Closed johnd0e closed 5 years ago

johnd0e commented 5 years ago

I have an idea how we could facilitate our development process, delivering new features faster, while keeping master branch clean, neat and stable.

So, what I propose:

  1. master branch is only for stable builds with well-tested (in other branches) changes. Push to master means (auto)deploying new stable build.
  2. For every new set of changes we use separate feature-branches (PRs), which should be made vs master. PRs are keeping open until final merging into master (it's possible to squash/rebase them to make history cleaner).
  3. development branch is for early-merging of feature-branches, as they go. 'Early' means that we could merge them not well-tested, then fix, remerge, (force)reset, etc, not bothering to keep history clean.
    • Cons: could be unstable.
    • Pros: features delivered earlier, which in turn let to test them thoroughly.
  4. After we decide that feature is ready - we merge it into master, closing PR.

So instead of current test builds we get development builds (title is not important).

Main difference: there we could play with features more bravely, because we would not care about secondary things (such as clear history).

modos189 commented 5 years ago

Now with friends I have long argued about the use of git flow. I don't think he's right for us. Also I don't understand you proposed a schema, it also seems unnecessarily confusing.

We can use Releases ( https://github.com/IITC-CE/ingress-intel-total-conversion/releases ). No, we NEED use Releases to build releases, since we need to provide the changelog for Google Play and Telegram channel. At the same time, really, I hasten to close PR, then not to forget about some of them.

What if we continue to use master for test builds but create a task with a list of changes needed for the new release? When all desired changes are made and merged, Release will be called?

johnd0e commented 5 years ago

Also I don't understand you proposed a schema, it also seems unnecessarily confusing.

Well, perhaps my explanation was not good. I'll try to explain my motives. Currently I'm trying in parallel to develop several features. All of them are work-in-progress, and not good enough to be merged into master. But the features are still usable.

Obviously, we can merge them right now, and fix later (if needed). But using master I feel 'bounded'. Because I know that at some point I'll have to rewrite that experimental code. Obviously I cannot 'undo' master, so there left much 'waste' commits. And I care about it because a year later me, or you, or some other developer will have to dig through dirty unclear history.

I have no such issue while I stay in separate 'feature'-branch, as I am free to do with it all that I want, even hard reset to beginning. But I have many such branches, and if I'll wait until all of them get ready to merge into master then we will not see the features a long time.

That's why I propose developer branch, where we could freely experiment with code, at the same time quickly deliver features to test builds. Well, we can try this workflow, and may be it'll appear not so confusing.

Don't you want to try? At any point we can delete that development branch and return to linear workflow in master.

johnd0e commented 5 years ago

No, we NEED use Releases to build releases, since we need to provide the changelog for Google Play and Telegram channel.

Well, I do not see any issue here. Sure we can use Releases (though I am not sure that it could help).

At the same time, really, I hasten to close PR, then not to forget about some of them.

See #55, and try Projects section. It really could help.

modos189 commented 5 years ago

also we can use milestones https://github.com/IITC-CE/ingress-intel-total-conversion/milestone/1

johnd0e commented 5 years ago

What if we continue to use master for test builds but create a task with a list of changes needed for the new release? When all desired changes are made and merged, Release will be called?

Well, public roadmap will be good, we could use milestones, and Projects for that purpose. But again: it is not why I create this issue.

modos189 commented 5 years ago

So, do you propose to develop in develop branch, and then, when we believe that everything is ready, squash and merge all the changes to master?

johnd0e commented 5 years ago

when we believe that everything is ready,

Some clarification: when we believe that one feature (or several features) is ready. In the same time other features could not be ready and stay waiting next release.

squash and merge all the changes to master?

Squash only when necessary. E.g. if our feature branch contains 10 good distinct commits and 5 fixups, then we need to squash only fixups, and after rebase feature will merge into master as 10 commits.

modos189 commented 5 years ago

It sounds useful, but then the master branch does not apply to test builds or releases. Builds will not be built.

johnd0e commented 5 years ago

but then the master branch does not apply to test builds or releases.

master branch will apply for stable releases. development branch should apply for test builds.

I suppose we can setup travis for both tasks.

modos189 commented 5 years ago

But you say

[merge in master] when we believe that one feature (or several features) is ready.

For some projects, it's cool to release very often, as each feature is ready, but is it suitable for us?

johnd0e commented 5 years ago

Well, I do not think that it would be often. Ready == tested, so time needed for every feature. Not 1 day, and may be not 1 week.

And, in the end, we choose manually which feature should go to release. So it can be once a month, or whatever you decide.

modos189 commented 5 years ago

How in master merge only ready features, but not merge not ready? If all they are already merged in develop

johnd0e commented 5 years ago

In development we merge feature branches, not PRs.

In master we can finally merge by resolving PRs.

modos189 commented 5 years ago

Well, let's try this.

johnd0e commented 5 years ago

Ok, I will play with this flow and report here.

johnd0e commented 5 years ago

So, suppose we've already setup development branch. Then, to integrate commits from some PR (e.g. #60) we can use commands:

git checkout development
git pull origin pull/60/head

And repeat it after each PR's update.

johnd0e commented 5 years ago

For some projects, it's cool to release very often, as each feature is ready, but is it suitable for us?

It's possible to setup travis-ci to make stable builds only for tags/releases. And even more automate: built assets could be attached to https://github.com/IITC-CE/ingress-intel-total-conversion/releases.

johnd0e commented 5 years ago

The better name for subject branch would be not development but rather test or even test-build(s).

modos189 commented 5 years ago

I suggest after this release to merge ready PRs in test branch. Because ready open PR is awful to observe)

johnd0e commented 5 years ago

Sure. But note that PRs still stay open until merged into branch they've were created upon - master.

modos189 commented 5 years ago

Now we can't call master builds as releases. Release requires creation of Release in github. From there, with description, release will be sent to Google Play and Telegram channel.

Way 1: master builds == release-candidates. Aren't there too many entities? Test builds, release-candidat build, release build. I think it's too much for us.

Way 2: master builds == minor changes, release builds == major changes. We can combine that with #63: minor changes are updated by means of IITC-Mobile, major changes require updating IITC-Mobile.

johnd0e commented 5 years ago

Release requires creation of Release in github. From there, with description, release will be sent to Google Play and Telegram channel.

Great.

I think it's too much for us.

Agree.

master builds == minor changes, release builds == major changes.

Not necessary 'major' changes. I think we can make new release as soon as we just feel that 'it's time'.

minor changes are updated by means of IITC-Mobile, major changes require updating IITC-Mobile.

Not sure that it should depends on IITC-Mobile. And as for versioning I'd prefer this concept: https://semver.org/:

MAJOR version when you make incompatible API changes, MINOR version when you add functionality in a backwards-compatible manner, and PATCH version when you make backwards-compatible bug fixes.

modos189 commented 5 years ago

Not sure that it should depends on IITC-Mobile.

Releases are useful because they allow specify a description. If do not upload new .APK, then there is no need to write a description, and it is sufficient to use maser builds.

That's why I suggest making small releases from master and large (accumulating released changes) from Releases.

johnd0e commented 5 years ago

Yes, I understand. And there are still different options.

  1. Descriptions are nice, so we may specify them even without apk. Also we can attach built iitc's snapshot in every release.
  2. Apk can be attached anyway (even if there are no app-specific changes), as it contains updated scripts.
modos189 commented 5 years ago

We can make descriptions for each release. But then master builds is not used

johnd0e commented 5 years ago

Let me summarize:

master is our main branch, it is base for all PRs. And yes, earlier we agreed that make 'stable' build with every commit would be excessive. So for master it's better to build tags/releases only.

johnd0e commented 5 years ago

Ok then. Just pushed new branch https://github.com/IITC-CE/ingress-intel-total-conversion/commits/test-builds.

git pull --no-ff upstream pull/74/head
git pull --no-ff upstream pull/77/head
git pull --no-ff upstream pull/81/head
git pull --no-ff upstream pull/78/head
git pull --no-ff upstream pull/83/head
git pull --no-ff upstream pull/84/head
git pull --no-ff upstream pull/72/head
git pull --no-ff upstream pull/79/head
git pull --no-ff upstream pull/86/head
git push upstream test-builds:test-builds

Please test all included features. I've create the project to easy track all this: https://github.com/IITC-CE/ingress-intel-total-conversion/projects/1

Feel free to add new cards and move them around.

Could you setup auto build/deploy from new branch?

johnd0e commented 5 years ago

git pull --no-ff upstream pull/96/head #96 git pull --no-ff upstream pull/81/head #81 (again) git pull --no-ff upstream pull/92/head #92

modos189 commented 5 years ago

I think the best option is: push to master leads only to updating the release on site, but creating a GitHub Release also leads to attaching files to Release and sending update to Google Play/etc

johnd0e commented 5 years ago

Well, I'd still prefer test builds from test branch. It let us to play with experimental code, which is not in 'master' (and possibly never be there).

modos189 commented 5 years ago

Yes. branch test-builds → Test build on site branch master → Release build on site GitHub Release → Release build on site, release build in assets, and Google Play/Telegram Channel/etc

johnd0e commented 5 years ago

branch master → Release build on site

Do we need this separate step? IMHO it's excessive, and even can lead to broken 'stable' builds on site.

Example: we prepare release by sequentally merging 10 PRs. And unexpectedly run into some conflict.

modos189 commented 5 years ago

Maybe. But I want to be sure that release build is really working before creating GutHub Release

johnd0e commented 5 years ago

Sure. But as all individual changes supposed already tested in test-builds I think it be enough just test merged master locally (not deploying it to site).

modos189 commented 5 years ago

Correctly. I didn't think about local testing. I will update my PR https://github.com/IITC-CE/ingress-intel-total-conversion/pull/97