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

Allow different templates for secondary literature #1043

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

fjorba
Copy link
Contributor

@fjorba fjorba commented Dec 15, 2020

This patch is the second version of the first step towards having different
templates for secondary literature. It synchronises some code and
configuration files from sources so now, when creating a new record, appears
a menu with a single choice, the legacy template.

This version takes into account the catalogues to publications rename and it
adds the locale entries compatible with the i18n reorganisation.

There are yet no new fields (like record type) or tag changes.

This patch is the second version of the first step towards having different
templates for secondary literature.  It synchronises some code and
configuration files from sources so now, when creating a new record, appears
a menu with a single choice, the legacy template.

This version takes into account the catalogues to publications rename and it
adds the locale entries compatible with the i18n reorganisation.

There are yet no new fields (like record type) or tag changes.
@fjorba
Copy link
Contributor Author

fjorba commented Dec 15, 2020

This patch updates and replaces #1017.

@@ -246,6 +246,9 @@ de:
individual_entry_threatise_printed: "Theoreticum, gedruckt - Teileintrag der Sammlung"
single_threatise_printed: "Theoreticum, gedruckt - Individualeintrag"

legacy_templates: "LEGACY TEMPLATES"
Copy link
Contributor

@ahankinson ahankinson Dec 15, 2020

Choose a reason for hiding this comment

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

We are trying to not put "placeholders" in the language files, so every new entry should be translated into its appropriate language before being added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just followed @xhero advice: #1017 (comment)

So, which is the preferred way now?

@@ -1,14 +1,14 @@
ActiveAdmin.register Publication do

Copy link
Contributor

Choose a reason for hiding this comment

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

Having these changes in this patch makes it difficult to review this change.

Copy link
Contributor Author

@fjorba fjorba Dec 15, 2020

Choose a reason for hiding this comment

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

I understand, but Emacs strips those extra spaces for me. In fact, there are so many in Muscat that it is difficult for me to stop Emacs to strip them everywhere else. Shouldn't be a policy in Muscat, like in other projects, to strip them before commiting upstream? I know that there are git hooks for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the other hand, I have just seen that from the Github web interface it is possible to hide whitespace changes, using the icon in the middle of the Review changes button line.

@fjorba
Copy link
Contributor Author

fjorba commented Jan 5, 2021

May I do anything to help the approval of this patch?

@xhero
Copy link
Contributor

xhero commented Jan 12, 2021

Sorry! I'm a bit late on this track, I will follow a review shortly!

@xhero
Copy link
Contributor

xhero commented Jan 20, 2021

Everything in the patch seems ok to me, for the moment we can keep then placeholder since they are in the "interface" language file.
I only have one question: where do you store the resulting record_type? In Source there is a dedicated field, but I can't find one here.

@fjorba
Copy link
Contributor Author

fjorba commented Jan 20, 2021

I only have one question: where do you store the resulting record_type? In Source there is a dedicated field, but I can't find one here.

It's requested in #1028 (I've also put some other requests on hold, because maybe there are better solutions).

@xhero
Copy link
Contributor

xhero commented Jan 20, 2021

ok, for the moment I would add a record_type as you suggest, I can also rename title to description

@fjorba
Copy link
Contributor Author

fjorba commented Jan 20, 2021

Could you also add name to short_name to this shorter list, please? The reasons are explained in #1027, and I understand that being, as it is, an internal identifier, will not hurt anybody.

@xhero
Copy link
Contributor

xhero commented Feb 17, 2021

Sorry for the silence - I had to take a small detour for other projects, I will add those fields in 7.1 (and in the next dev branch) so we can merge this.

@fjorba
Copy link
Contributor Author

fjorba commented Dec 11, 2023

Can I do anything else for this (and my other pending PR) to be included upstream, @xhero? I'm applying those patches to my sites for each new Muscat release, and we are using them daily (although not in production yet). Thanks.

@fjorba fjorba marked this pull request as draft February 16, 2024 16:48
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.

3 participants