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

Help wanted to finish refactoring services 🏆 #2863

Closed
calebcartwright opened this issue Jan 25, 2019 · 20 comments · Fixed by #3367
Closed

Help wanted to finish refactoring services 🏆 #2863

calebcartwright opened this issue Jan 25, 2019 · 20 comments · Fixed by #3367
Labels
good first issue New contributors, join in! service-badge Accepted and actionable changes, features, and bugs

Comments

@calebcartwright
Copy link
Member

calebcartwright commented Jan 25, 2019

Refs #1358 and #963

Over the last year+, a lot of great work has been done to refactor many of our legacy service classes to the new service architecture. There's still quite a few remaining services in need of refactoring though.

Legacy service refactors are highly valuable contributions that are greatly appreciated, and a great way to contribute to Shields!

legacy services needing refactor

How can you help?
All of our service classes are listed in this spreadsheet, and Column C indicates whether the respective service has been refactored to the new service architecture (green check) or is still using the legacy service model (red).

Locate the row for the service you'd like to refactor, and volunteer by adding a comment (in Column D) that includes your GitHub username so everyone can see who is working on each service.

Once you've updated the spreadsheet you're ready to get started! We've got a great tutorial with guides and tips for refactoring services.

Other useful documentation for contributing:

You can reach out to us on this thread and/or Discord with any questions!

@calebcartwright calebcartwright added good first issue New contributors, join in! service-badge Accepted and actionable changes, features, and bugs labels Jan 25, 2019
@chris48s chris48s pinned this issue Jan 25, 2019
@paulmelnikow paulmelnikow changed the title Finish Refactoring Services 🏆 Help wanted to finish refactoring services 🏆 Jan 25, 2019
@PyvesB PyvesB unpinned this issue Feb 6, 2019
@PyvesB PyvesB pinned this issue Feb 6, 2019
@paulmelnikow
Copy link
Member

According to the refactoring report, there are 69 legacy services left. 5 of those are in PR which leaves 64.

How about we set a goal for wrapping these up? We could aim for the end of March. That would mean merging roughly two of them per day, which seems not too aggressive. It's a target I think we could reach.

I kinda want to celebrate this milestone!

@calebcartwright
Copy link
Member Author

According to the refactoring report, there are 69 legacy services left. 5 of those are in PR which leaves 64

I did not realize we had a discrepancy between the number remaining badge (above and in the Readme)!I wonder if I broke something when I split out additional rows in the report for some of the service classes

How about we set a goal for wrapping these up?

I'm good with that 👍 In particular you and @chris48s have really been burning through them lately 🔥

@paulmelnikow
Copy link
Member

The readme badge uses the GitHub search count, which I believe counts files, so it undercounts files with multiple services but does credit you for finishing a whole file. The script counts registerLegacyRouteHandler which gives 69.

@paulmelnikow
Copy link
Member

What a big week! 🚒 🙌

Files: 48 -> 39
Classes: 69 -> 56

@calebcartwright
Copy link
Member Author

There's a whole lot of 🍏 📗 💚 in the spreadsheet now!

@calebcartwright
Copy link
Member Author

Well, we didn't hit the end of March target but we're still making good progress.

I have a feeling that the Codeclimate refactor is one of the more daunting ones remaining

@paulmelnikow
Copy link
Member

paulmelnikow commented Apr 13, 2019

Scary close!

Files: 39 -> 17
Classes: 56 -> 23

Four more in review, as well!

@Daniel15
Copy link
Member

Nice work on this!! 😃

@paulmelnikow
Copy link
Member

Two years in the making!

Screen Shot 2019-04-13 at 3 44 46 PM

I wonder if we could merge the last one on the 26th – that'd be cool!

@calebcartwright
Copy link
Member Author

I like the 26th as our target goal for finishing 👍

@paulmelnikow
Copy link
Member

paulmelnikow commented Apr 13, 2019

Would it be helpful to create a pared down list for the home stretch and try to have folks claim them?

It's not many that aren't started yet!

Service User/Notes
codeclimate @paulmelnikow
github commits since @paulmelnikow
github downloads @paulmelnikow
github tag @paulmelnikow
jenkins build @calebcartwright
jenkins plugin version @PyvesB
jenkins tests @calebcartwright
maven central @Prouser123
php-eye php version @calebcartwright
scrutinizer @calebcartwright
travis php version @paulmelnikow
vaadin directory @chris48s

@paulmelnikow
Copy link
Member

Would be fun to get all hands on deck in the wrap up. Anyone want to claim one? @badges/shields-core @Daniel15

@calebcartwright
Copy link
Member Author

I didn't add my name on the php-eye version one in the spreadsheet as I expect it will need to be deprecated, refs #3264

The service/our badges have been broken for ~2 and half weeks now, with no end in sight.

@calebcartwright
Copy link
Member Author

kudos for codeclimate @paulmelnikow! 🎉

@chris48s
Copy link
Member

Vaadin directory is currently hard to work on because its difficult to get a test run to complete cleanly. See:

With GH tags and downloads, we should probably just port those as they stand so we can get the refactoring work done and then tackle the work discussed in #3144 (comment) (which I think is the correct way to go) as a different issue.

@PyvesB
Copy link
Member

PyvesB commented Apr 15, 2019

Slightly low on bandwidth with the Easter vacations coming up, but I'll take care of at least Jenkins Version. 😉

paulmelnikow added a commit that referenced this issue Apr 16, 2019
Attacking this in two pieces for ease of review. The legacy implementation for coverage is still there, though I disabled it via the route. That whole file will be removed in the next PR.

Ref #2863
paulmelnikow added a commit that referenced this issue Apr 23, 2019
For consistency with other download badges, I changed some formatting:

-  **downloads | 24k total** -> **downloads | 24k**
- **downloads | 3k** -> **downloads@latest | 3k**
- **downloads | 3k v0.29.0** -> **downloads@v0.29.0 | 3k**

Ref #2863
paulmelnikow added a commit that referenced this issue Apr 24, 2019
The change in `expectBadge` prints a more helpful error when `message` is empty.

Ref #2863
paulmelnikow added a commit that referenced this issue Apr 25, 2019
@calebcartwright
Copy link
Member Author

And then there was one.. 🥇 #3266 will close out the service refactoring!!!

@Daniel15
Copy link
Member

Sorry I didn't have time to work on this, but really nice work everyone! :D

@PyvesB
Copy link
Member

PyvesB commented Apr 27, 2019

Small Tweet to celebrate the achievement:
https://twitter.com/Shields_io/status/1122249863477985280

🍾

@chris48s
Copy link
Member

This is an incredible milestone to have achieved and a poetic date on which to have finally completed it. Beyond marking the occasion with a single 🎉 emoji, I have a couple of thoughts on rounding out the process:

Retrospective

Maybe a nice way to close out this process would be to do some kind of fairly loose written retrospective. There might be scope to turn it into a blog post or something collating our thoughts on what we did well, what we did badly, what we learned from it (I'm hoping that some of us have learned something, rather than just grinding away at it for 2 years 😆 ), and maybe extrapolating some transferable lessons that someone embarking on refactoring a large codebase might be able to take from it. It would also be an opportunity to shout out the wider group of contributors who have chipped in to help with the refactor.

As a first step, I've set up a document at https://docs.google.com/document/d/1t6FmldEHKFB_9WjLn1TtWtETMT6f69wYQszTgzYtPPE It is publicly viewable and I think all the core team should have access to edit it. Its in the same shared folder as the roadmap etc, but ping me or @paulmelnikow if you want to contribute and can't edit. I've started off by collecting a few of my own thoughts on the process. It would be great to get a few bullet points or something from the rest of the maintainers to feed into this. I'm sure once we've collated some different perspectives we'll have enough material to turn it into...something. Dunno - maybe it will all end up being too insanely specific :) Either way, I look forward to reading your thoughts on the process.

What's Next

This is a great time to take a step back and enjoy the achievement of finishing this huge project, but its also a moment to think about where the project goes next. Beyond the daily grind of bugfixes and failing tests, what are the next milestones we should be working towards? Perhaps the upcoming days and weeks are a good time to revisit the roadmap and think about where we should focus next both in terms of our role as a group of maintainers and our asks for the wider community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue New contributors, join in! service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants