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

DOCS Contributing code cleanup #4285

Merged
merged 1 commit into from
Jul 7, 2015

Conversation

jonom
Copy link
Contributor

@jonom jonom commented Jun 15, 2015

Fixed a few typos and formatting issues, and made git workflow diagram easier to read by hyperlinking it to itself. Also brought down a few lines from 3 branch that maybe should have targeted 3.1 and been merged up - is it okay to do that or should I take those lines out?

@chillu
Copy link
Member

chillu commented Jun 16, 2015

Nice, thanks! Some shocking spelling mistakes in there ... we need better spell checking in our markdown editors obviously ;) I want to avoid version specific docs as much as possible, it adds to maintenance burden and is likely going stale. Could you rephrase your hint to a normal sentence saying something like "Please adjust to the SilverStripe version you're targeting. The current version can be found on http://www.silverstripe.org/software/download/"

@jonom
Copy link
Contributor Author

jonom commented Jun 16, 2015

Yeah that's a good idea. The 3 branch has that exact sentence already except it says 3.2, so if I change it here will it get merged up?

@chillu
Copy link
Member

chillu commented Jun 16, 2015

Yeah that's the idea - not immediately, but periodically

@jonom jonom force-pushed the docs-contributing-cleanup branch from 9d7abfd to 16d9e07 Compare June 16, 2015 21:54
@jonom
Copy link
Contributor Author

jonom commented Jun 16, 2015

You know looking over this content a bit more I think it is too heavily focused on git commands and not enough on code quality. Think this should maybe be split in to a few pages and start off with a table of contents / check-list something like this:

So you want to contribute to SilverStripe?

Great! Contributing to the Framework, CMS or SilverStripe modules is a great way to learn more about and improve the tools you use everyday, for everybody's benefit.

We use github to coordinate code contributions. If you're new to git don't worry, we'll walk you through the commands in the links below.

To make a contribution:

  1. If you're working on a major change, seek feedback from the community and core committers first
    Make sure your proposal is solid before you spend time working on it (You can skip this if you want to fix a small bug or typo)
  2. Review SilverStripe's coding conventions
    Writing clean code off the bat will mean your contribution gets accepted quickly
  3. Fork the project you want to contribute to and start working in a new branch
    Branching allows you to experiment freely with code changes while keeping a clean base
  4. Create a pull request
    Members of the core committers team will review your patch and give you feedback about any tweaks that are needed before merging

For more information on git check out these (links to tutorials and guis like Tower)

@chillu
Copy link
Member

chillu commented Jun 17, 2015

Yeah, that sounds like a good improvement. Just be aware there's already a separate "landing page" at http://www.silverstripe.org/community/contributing-to-silverstripe/, so we should ensure we don't produce too much overlap. Do you want to base a pull request on mine so they can be merged in sequence? Probably the easiest way to collaborate on it. I won't force push in that case.

@jonom
Copy link
Contributor Author

jonom commented Jun 17, 2015

Good point, hadn't seen that page. Happy to have a look at restructuring this section a bit but probably will be next week.

Do you want to base a pull request on mine so they can be merged in sequence?

Sorry lost me there - do you have an open PR for the same file somewhere?

@chillu
Copy link
Member

chillu commented Jun 17, 2015

do you have an open PR for the same file somewhere?

If you start to restructure the doc in this pull request without waiting for said pull request to be merged, we'll get a lot of conflicts once this request is merged. So either you wait until somebody merges this request (@tractorcow?), or you base your own pull request on my branch which will avoid later merge conflicts.

@jonom
Copy link
Contributor Author

jonom commented Jun 17, 2015

Think I get it. I will do the waiting option :)

Going forward with SemVer I'm assuming there won't be separate versions of docs for minor releases - just one version of docs for each major release right? And same with branches, I'm assuming there will be no 3.2 or 3.3 branch of SilverStripe, just tags on the 3 branch. Is that right? Probably going to be a nightmare to maintain otherwise.

@tractorcow
Copy link
Contributor

@chillu I'm also a bit confused (as per normal hah)... which is your branch? Can you link it?

@jonom
Copy link
Contributor Author

jonom commented Jun 17, 2015

Heh the 'my branch' part is what stumped me, but luckily 'waiting' provided an easy out 😊


# [make sure all your changes are committed as necessary in branch]
# make sure all your changes are committed as necessary in branch
git fetch upstream
Copy link
Contributor

Choose a reason for hiding this comment

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

these two commands can be simplified to $ git pull --rebase upstream master

@jonom
Copy link
Contributor Author

jonom commented Jun 18, 2015

Hey @dhensby thanks for the comments - most of the stuff you've commented here I just pulled down from the 3 branch so changing it might cause some confusing merge conflicts later I guess. Maybe I shouldn't bother with this at all - what is the policy for maintaining documentation for older versions of SilverStripe (which 3.1 is about to become?)? I noticed that 3.0 and earlier docs have a message stating that the docs may be out of date ant not maintained, so maybe it is a waste of effort for me to submit any changes to 3.1 at this point and I should just update this stuff on the 3 branch instead?

@dhensby
Copy link
Contributor

dhensby commented Jun 18, 2015

I think this should go in, it won't cause a merge problem.

You should apply patches to the LOWEST version they apply to. so 3.1 is the best place. they should then be merged up into 3.2 and 3.3 and 4

@jonom
Copy link
Contributor Author

jonom commented Jun 18, 2015

Cool. @dhensby maybe you can confirm for me - if we're adopting SemVer properly going forward won't that make it redundant (and just a pain) to have separate docs for 3.2 and 3.3? I was under the impression that one of the advantages of adhering to SemVer is that you only need one branch for each major version because everything added to it will be backwards compatible. So if that assumption is correct there should be just a 3 branch of SilverStripe with tags with a linear progression e.g. 3.2.0, 3.2.1, 3.2.2, 3.3.0, 3.3.1 etc. then a 4 branch following a similar pattern. I think with each release the decision to make it a patch vs minor release is only one of semantics.

Before adopting SemVer all SilverStripe 'minor' releases would definitely have been considered major releases from a SemVer point of view so they needed to be maintained as separate branches. But that shouldn't be the case anymore, so shouldn't we be changing our approach to releases and really embracing SemVer? I think it's going to really slow things down and hamper progress if we try to maintain individual branches for minor releases.

@tractorcow
Copy link
Contributor

@jonom one branch is for each minor version actually (3.1. 3.2, etc). Otherwise we couldn't release bugfixes that affected only older versions (which we still have to support). It would be nice to just force users to update to the next minor version each release, but that's a future thing not a now thing, and it's a step we'd only take if the community supported it.

Maybe once we increase the velocity of our releases it'll be easier to do and we can start doing some rotation of our older branches, as well as having fewer new branches. :D but that's not a call we want to make just yet.

@jonom
Copy link
Contributor Author

jonom commented Jun 22, 2015

@tractorcow I feel pretty strongly that this is the wrong approach. It seems like we're trying to continue doing things 'the old way' which is counter-productive and likely to create a ton of extra work and a dis-incentive for contributions.

The adoption of SemVer should allow us to regularly (and casually) update the framework with new features that are not breaking changes. It should be safe for all users to update to the newest minor version so there should be no reason for anyone to need to stay on e.g. 3.2 vs 3.3. It shouldn't be unreasonable to think that we could get to SilverStripe version 3.15 before 4.0 ships, but it would be completely unreasonable to maintain 16 separate branches of the framework and docs to go with it.

If we maintain separate branches for 3.2, 3.3, 3.4 I think we can kiss the idea of 'regular updates' goodbye, because there would be an incentive to keep the number of minor releases as low as possible, thus creating less merge conflicts and less versions of the docs.

We don't maintain separate branches for 3.1.1 and 3.1.2, even though the upgrade between these would have been the equivalent of at least a minor and maybe a major release under SemVer (check out the changelog), so it seems crazy to me to plan to maintain separate branches for minor releases going forward. I'm really concerned that it's going to slow things down and inhibit progress, because we'll get bogged down in the commitment to maintain legacy releases that no one should need to use.

Maybe once we increase the velocity of our releases it'll be easier to do and we can start doing some rotation of our older branches, as well as having fewer new branches. :D but that's not a call we want to make just yet.

I think this is a backwards approach, because making the call to make minor and patch releases tags only is what would enable us to increase the velocity of releases. I think it would be quite empowering for developers. Note that the SemVer spec indicates that if a breaking change is accidentally introduced, it should be reverted in a following release, so this gives us a way to repair things if we stray from SemVer from time to time, surely making separate branches for minor releases redundant.

Really think there needs to be a paradigm shift for all developers in our expectations about what a minor release constitutes - not just about what is safe to put in to a minor release, but how legacy versions and docs are maintained and where developers can expect to find the latest bug fixes. Going forward developers should think of minor releases similar to the way they used to think of patch releases - in fact minor releases now should be much safer than patch releases used to be. A minor release shouldn't need to come with a bunch of fanfare or a new set of docs - we should be able to simply go, "okay there's some updated code on the 3 branch, time to do a release. If only bug fixes then update patch no. by 1, if new non-BC features update minor release no. by 1 and reset patch no. to 0".

Keen to hear your thoughts on this @tractorcow and if you think my arguments have any merit maybe I should bring this up on the dev list? Apologies if this has been discussed in depth offline and I'm missing something obvious, but I feel if this is the point where we adopt SemVer the release process should embrace that system fully and permit a fluid release cycle rather then inhibit it. Or if 3.x is the wrong time it would be good to get a discussion going now so we can reach a consensus about the release process ready for 4.0.

p.s. I realise we'd need to still maintain a 3.0 and 3.1 branch going forward as these are not SemVer compliant but I think 3.2 and onwards should just be on the 3 branch and releases simply tagged.

@dhensby
Copy link
Contributor

dhensby commented Jun 26, 2015

I completely agree with @jonom in terms of what the approach needs to be in terms of frequency of tags.

We need to make tagging releases trivial and regular so patches can be released quickly and new features added regularly.

I'm not sure we'll be seeing 3.15 ( :P ) but SemVer should allow us to maintain 1 set of docs for each major version.

Once we are able to release quickly and with little effort, this won't have as much internal resistance and will make sense to everyone.

@jonom
Copy link
Contributor Author

jonom commented Jun 26, 2015

🌹 Thanks for backing me up @dhensby

Once we are able to release quickly and with little effort, this won't have as much internal resistance and will make sense to everyone.

Are there any particular processes that need to change to facilitate rapid releases and one branch only per major version, or is this stuff already underway and just needs some time? Think we should start working towards this goal now but I'm not informed enough to know if and what needs to be changed. At a minimum I think at the time 3.2 stable is released we should inform developers how the release process and docs structure will change in the future, so developers have clear expectations about where SemVer is taking us.

@dhensby
Copy link
Contributor

dhensby commented Jun 26, 2015

Well, the release process includes extended testing, writing release announcements and other manual work. It needs to be automated far beyond its current state.

@sminee is doing some work to that extent, but we need to automate the entire thing IMO

Then, in theory, we can release patches every week if we wanted to

@dhensby
Copy link
Contributor

dhensby commented Jun 28, 2015

@jonom please can you adjust the code snippets as per my suggestions, then I'll pull this in...

@jonom jonom force-pushed the docs-contributing-cleanup branch 2 times, most recently from f873bac to 82c6cb7 Compare June 28, 2015 19:50
@jonom
Copy link
Contributor Author

jonom commented Jun 28, 2015

@dhensby that's done now. Also added a summary, note about fixup vs squash and standardised the white space a bit more.

@jonom
Copy link
Contributor Author

jonom commented Jun 28, 2015

@dhensby @tractorcow should I open a discussion on the dev list summarising the content of #4285 (comment)?

@jonom jonom force-pushed the docs-contributing-cleanup branch 3 times, most recently from abcbc83 to 916dc32 Compare June 28, 2015 20:10
Fixed a few typos and formatting issues, and made git workflow diagram easier to read by hyperlinking it to itself. Also included a few lines from 3.2 branch.
@jonom jonom force-pushed the docs-contributing-cleanup branch from 916dc32 to a39c2bd Compare June 28, 2015 20:19
@SpiritLevel
Copy link
Contributor

@jonom @dhensby @tractorcow @camfindlay @chillu @willmorgan

I raised related issues in https://groups.google.com/forum/#!topic/silverstripe-dev/jG3Un5hvYVM

There seem to be some pretty fundamental issues involved here around SS development, contribution, release, and documentation that need sorting.

My being new to SS allows me to look at things with a fresh pair of eyes, and I strongly support the simplifying and clarifying of these processes. I think it would be very helpful to discuss these in more detail. I offer to play the role of "secretary" and document the discussion, note conclusions, and follow up on actions to be taken.

I think it is also useful to see what others are doing:

http://cakephp.org/
http://www.codeigniter.com/
https://symfony.com/
http://laravel.com/
http://www.yiiframework.com/
http://www.zend.com/

@jonom
Copy link
Contributor Author

jonom commented Jun 28, 2015

@SpiritLevel awesome to see such commitment from someone who is only new to SilverStripe! Look forward to seeing what you collect. Maybe collect input on the dev list and release conclusions as an RFC? A few goals I'd like to see included:

  • Reduce overlap (and maintenance commitment) - only document topics once wherever possible. If a topic is covered in detail in a Lesson, only provide a brief introduction for it in the Developer Docs and say "Go to lesson xx to learn about yy" (I haven't watched the lessons yet but they look awesome and the best resource for beginners, so should promote that content first whenever possible). If a function or class is documented well at the API level, link to specific pieces of API documentation instead of saying the same thing a different way in the Developer Docs.
  • Only one version of developer docs per major version Going forward there should not be separate docs for each minor version. This is consistent with (or exceeding actually) the level of documentation versioning applied until now to non-SemVer releases.
  • No broken docs links 301 redirects for any docs when the URL changes. I have very often gotten a 404 while exploring the docs.
  • Assume knowledge / Limit scope creep In general assume a working knowledge of technologies like PHP, git, SQL, OOP, Packagist etc. but include a 'Required Knowledge' section or similar which assumes no knowledge at all and links to quality learning resources for these topics, while also outlining any applicable SilverStripe-specific concepts or exceptions.

The docs have come a long way in the last couple of years but there is always room for improvement.

@jonom jonom closed this Jun 28, 2015
@jonom jonom reopened this Jun 28, 2015
@jonom
Copy link
Contributor Author

jonom commented Jun 28, 2015

Come on Travis... You can do it...

@SpiritLevel
Copy link
Contributor

First, even before an RFC or addressing @jonom 's very specific suggestions, I think we to need to get a sufficiently large and committed working group of people to take this on. We need a critical mass of people with SS Ltd leading and backing it.

Second, I think we should, as much as is feasible, collect existing comments into one location to consolidate it all. For instance, @camfindlay has already done heaps of data collection here:

https://groups.google.com/forum/#!topic/silverstripe-dev/3X8kZVSqPiY

There was also, until recently, a very quiet google group on SS docs but it looks as though it has disappeared (several voted to kill the poor thing!). Perhaps, @camfindlay, this should be resurrected ? Or, (which only reinforces my point below) have I missed it somewhere ? Are there other sources where valuable opinions/suggestions are provided but are lost in obscurity ?

More generally, there seems to be a huge fragmentation of valuable information coming in via Github comments, Google group mailing lists, UserVoice, forums, etc....a handful of people read the comments at the time they are posted and then they fade into obscurity, relying on Google's indexes for any sort of permanence. The dual SS websites seem to further facilitate this fragmentation! Too much information spread out too thinly all over the place!

Third, I think some kind of "Communications Strategy/Policy" needs to be worked out/brainstormed as a roadmap for the group, for SS, and the wider community to follow. An approximate timeline would be good too. The approaches taken by other frameworks provide lots to think about and build upon. Again, this is where SS Ltd. backing and involvement would be crucial. Without SS Ltd. buy-in, I fear the docs will continue to be tweaked in a fragmented, uncoordinated way.

So, what do you think ?

@jonom
Copy link
Contributor Author

jonom commented Jun 29, 2015

@dhensby passed this time - should be good to go.

@SpiritLevel sounds like you have big plans! Think you'd be best off moving this discussion to a new topic on the dev list... maybe called 'Attacking Fragmentation' or something :)

@dhensby
Copy link
Contributor

dhensby commented Jun 29, 2015

@SpiritLevel I agree. The docs need an overhaul in my opinion and I think I was fairly vocal about that a while back when the team was discussing new IA for the docs.

I'd love to contribute to making the docs better or having a more focused community site, but I fear I don't have the time.

SS is a pretty huge system with a lot catered for, so docs aren't so simple to approach, there are many APIs and sometimes they are inconsistent or complex.

What timezone are you in? Perhaps we can have a chat.

@jonom I'm on my phone and haven't reviewed this since the last set of changes. I'll try to look soon

@SpiritLevel
Copy link
Contributor

@dhensby I am in Wellington :)

I am Skyping with a client at 9AM Wednesday, my time, but apart from that any time that suits you.

You're in London, right ?

We could also chuck ideas around on a google doc.

@jonom
Copy link
Contributor Author

jonom commented Jul 7, 2015

@chillu I saw elsewhere you said that 3.1 is bugfix only now - does that apply to docs and if so should I direct this to 3.2 instead? Also would love to hear your thoughts about minor release branches and docs under SemVer and whether this needs some discussion on the dev list.

@chillu
Copy link
Member

chillu commented Jul 7, 2015

@jonom I think docs improvements in 3.1 is fine, but I'd leave major restructuring for upcoming releases since it'll be too hard to consolidate otherwise. I agree with Damian that we should expose one doc version per minor release. We can introduce new features in minor releases, and don't want to a) confuse devs about feature availability in older minor releases or b) litter the docs with "since x.y" annotations which need to be cleaned up later. Having more releases selectable on doc.ss.org will require a UI change there, but that should be minimal. I always take readthedocs as an good example - they by default generate docs for every tag, and make selection easy (bottom left). Example: https://praw.readthedocs.org/

No broken docs links 301 redirects for any docs when the URL changes. I have very often gotten a 404 while exploring the docs.

@camfindlay I suspect the majority of this was caused by the docs restructuring late last year. Have you made any progress on fixing broken links from that? Maybe review Google Webmaster Tools or get webserver logs?

chillu added a commit that referenced this pull request Jul 7, 2015
@chillu chillu merged commit 31db269 into silverstripe:3.1 Jul 7, 2015
@camfindlay
Copy link
Contributor

@chillu @jonom we did some work on rewrites a while back and it's a time and resource thing for me (that I have minimal). I have posted a while back the top 50 urls from webmaster tools (silverstripe/doc.silverstripe.org#50).

Would be great to get some community help to whip up the rewrites and pull request. I'm happy to review an merge.

@jonom jonom deleted the docs-contributing-cleanup branch July 7, 2015 21:33
@jonom
Copy link
Contributor Author

jonom commented Jul 7, 2015

@chillu fair enough - yeah with an interface change I can see how it would be good to have access to all versions of the docs. Any thoughts on my opinion that maintaining separate branches for each minor release goes against the grain of SemVer, and may hamper progress and a rapid release schedule by creating extra work, as security patches and bug fixes would have to be applied to each branch? Is it not acceptable to expect devs to upgrade to a release that contains new non-breaking features in order to get bug fixes?

@camfindlay I could take a look at that some time, hopefully wouldn't take long to fix the majority of them. Is that list up to date? @SpiritLevel don't mean to dob you in but if that's something you'd enjoy feel free :) otherwise I'll make a note and try to get to it in the next few weeks.

@SpiritLevel
Copy link
Contributor

OK....I'll start on them, mixed in with client work :)

I'll post comments/questions to silverstripe/doc.silverstripe.org#50

@jonom
Copy link
Contributor Author

jonom commented Jul 7, 2015

Thanks man, feel free to bail out or just do a few, don't mean to pressure you!

@SpiritLevel
Copy link
Contributor

Happy to help...many hands make light work, and all....

@dhensby
Copy link
Contributor

dhensby commented Jul 8, 2015

Most devs should be locking to ~3.1 in their composer files. Only those that are particularly risk averse would lock onto patch versions (~3.1.13).

Although minor releases should be completely BC, there is higher risk assosiated with those upgrades.

Ultimately this is the point of LTS versions. We say, "if you're risk averse we will be applying security patches to 3.1 for 2 years and other versions will not have security patches added (except current latest release)

@jonom
Copy link
Contributor Author

jonom commented Jul 8, 2015

@dhensby totally agree that most devs would currently be locked on to ~3.1 and we would need to keep patching 3.0 and 3.1 because they weren't SemVer compliant.

However going forward, if your risk tolerance previously was for ~3.1 before we adopted SemVer, then your equivalent risk tolerance under SemVer should be ~3. Actually ~3 from 3.2 onwards should be less risky than ~3.1 ever was. So if we used to recommend installing SilverStripe with ~3.1 in your composer file then under SemVer we should recommend ~3, shouldn't we? To not do so would be moving backwards in my book.

This is what my whole argument is about, the risk factor has completely changed with the adoption of SemVer and so our release process and notions about what is safe to target in a composer file should be updated to reflect that change.

Ultimately this is the point of LTS versions. We say, "if you're risk averse we will be applying security patches to 3.1 for 2 years and other versions will not have security patches added (except current latest release)

Are you saying that 3.2 won't see security patches once 3.3 is released? That's exactly what I'm asking for, so maybe I'm just preaching to the converted :) I don't think the intentions for a future release process are documented anywhere which is why I'm curious about it.

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.

6 participants