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

Upgrade Ember and deps to 3.18/Octane #186

Merged
merged 15 commits into from
May 11, 2020
Merged

Conversation

raucao
Copy link
Member

@raucao raucao commented May 11, 2020

This PR doesn't upgrade components to be Glimmer components yet. It's already big enough as it is.

@galfert I'm not sure if you saw my chat message, but when you look at the tests you can find one helper failing (due to some kind of observer issues), and that one failure also seems to break the entire run loop of the Ember app when using it normally, thus freezing the app in place when it hits (which is when you look at contribution details). Hence the WIP.

I also removed all code relating to proposals on the way, as we're not using them at all at the moment. Thus, dragging the code along through big upgrades like this one didn't make sense to me. We can always retrieve it from Git whenever we need it again.

raucao added 10 commits May 9, 2020 17:07
Glimmer components don't have extra markup around them, so we don't need
the component JS files anymore. Also, we can easily ignore the icon
templates when linting now.
We're not using proposals at the moment, so it's just in the way.
These need more updates outside of the templates, like e.g. Glimmer
components and such.
@raucao raucao requested review from galfert and fsmanuel May 11, 2020 08:39
@galfert galfert self-assigned this May 11, 2020
The wrong property name caused an exception when trying to remove the observer.
@galfert
Copy link
Contributor

galfert commented May 11, 2020

Haha, now I wonder how it ever NOT caused an exception :) There was a typo in the property name in the function that removes the observer. I guess older Ember versions kept track of existing observers in a different way that didn't fail when trying to remove a non-existent observer.

@raucao
Copy link
Member Author

raucao commented May 11, 2020

wowitworked

@raucao raucao changed the title WIP: Upgrade Ember and deps to 3.18/Octane Upgrade Ember and deps to 3.18/Octane May 11, 2020
@raucao
Copy link
Member Author

raucao commented May 11, 2020

@galfert Thanks!

So this is ready to review/merge then.

@galfert
Copy link
Contributor

galfert commented May 11, 2020

I'm already reviewing it.

Copy link
Contributor

@galfert galfert left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Just left some minor comments.

But the kredits amount in the "Confirmed Contrubutions" list looks misaligned now:

kredits

app/components/contribution-list/template.hbs Show resolved Hide resolved
app/templates/dashboard.hbs Outdated Show resolved Hide resolved
codemods.log Outdated Show resolved Hide resolved
@raucao
Copy link
Member Author

raucao commented May 11, 2020

Thanks!

(I cannot reproduce the misalignment of the amounts.)

@raucao raucao merged commit 41cc37d into master May 11, 2020
@raucao raucao deleted the chore/upgrade_ember_and_deps branch May 11, 2020 11:19
@galfert
Copy link
Contributor

galfert commented May 11, 2020

(I cannot reproduce the misalignment of the amounts.)

Looks ok here now, too. I think it was the missing contribution-status class.

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

Successfully merging this pull request may close these issues.

None yet

2 participants