Skip to content

Replace 'types' with 'type'#6261

Merged
MichaelArestad merged 3 commits intofeature/settings-overhaulfrom
fix/types-to-type
Feb 2, 2017
Merged

Replace 'types' with 'type'#6261
MichaelArestad merged 3 commits intofeature/settings-overhaulfrom
fix/types-to-type

Conversation

@MichaelArestad
Copy link
Copy Markdown
Contributor

Fixes #6214

Now both types will be type

Not sure what the deal with module headings is, but I'm starting to think the one one feature/settings-overhaul is wrong.

@MichaelArestad MichaelArestad added the [Status] Needs Review This PR is ready for review. label Feb 2, 2017
@MichaelArestad MichaelArestad self-assigned this Feb 2, 2017
@beaulebens
Copy link
Copy Markdown
Member

The types=>type change looks good, but I think the module headings changes "conflict" with #6258.

@MichaelArestad
Copy link
Copy Markdown
Contributor Author

@beaulebens They do, but we can figure it out.

<span className="jp-form-toggle-explanation">
{
__( 'Enable Portfolio custom content types.' )
__( 'Enable Portfolio custom content type.' )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These periods should be removed.

@eliorivero
Copy link
Copy Markdown
Contributor

eliorivero commented Feb 2, 2017

The file module-headings.php is created during build so those changes in module-headings.php must be actually done in each module file header as shown here https://github.com/Automattic/jetpack/pull/6258/files#diff-fd02f3fe4ff6aae933fe9ad980a48557

Question @beaulebens should the periods at the end of these descriptions be removed? because some are used as labels for toggles.

Update: never mind, I saw Michael's PR https://github.com/Automattic/jetpack/pull/6259/files

@eliorivero
Copy link
Copy Markdown
Contributor

About the module-headings.php, I realized now that what you meant was that it changed without your intervention. That happens during a build. To avoid this I usually run gulp or gulp watch directly. Only when I know there's something to build (like due to an update in dops components) I run yarn build.

I've now restored the file and removed the periods at the end of the sentences.

@MichaelArestad
Copy link
Copy Markdown
Contributor Author

@eliorivero since it's generated, should we remove it from version control?

@jeherve jeherve added this to the Settings UI milestone Feb 2, 2017
@eliorivero
Copy link
Copy Markdown
Contributor

Yes, we should do it.

Btw this now is good to go 🐑

@MichaelArestad MichaelArestad added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 2, 2017
@MichaelArestad MichaelArestad merged commit 289bc9e into feature/settings-overhaul Feb 2, 2017
@MichaelArestad MichaelArestad deleted the fix/types-to-type branch February 2, 2017 16:10
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
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.

5 participants