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

feat: FC-0012 - add atlas support for cookiecutter-xblock #382

Merged
merged 1 commit into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions cookiecutter-xblock/cookiecutter.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
{
"project_desc": "My First XBlock",
"package_name": "myxblock",
"package_name": "my_xblock",
"i18n_namespace": "{{cookiecutter.package_name.split('_')|map('capitalize')|join('')}}I18n",
"github_org": "openedx",
"repo_name": "{{cookiecutter.package_name}}-xblock",
"tag_name": "myxblock",
"repo_name": "{{cookiecutter.package_name.replace('_', '-')}}",

Choose a reason for hiding this comment

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

this is changing behavior

before this change, the value of repo_name would be myxblock-xblock, now it's my-xblock

is this change intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is intentional, I see it as more convenient to have the repo name the same as the package name unless the creator decides otherwise. No technical reason or implication for that

Note: the template will not enforce anything. It'll just propose these as defaults, and the XBlock creator can change them on the spot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

another thing is that we have a new field i18n_namespace that will look nicer by this change

"tag_name": "{{cookiecutter.package_name|lower}}",

Choose a reason for hiding this comment

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

similar to the comment about repo_name - this will force _xblock into tag names (considering there is now an expectation that _xblock will be in the package name).

is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here

"version": "0.1.0",
"class_name": "MyXBlock",
"author_name": "Open edX Project",
Expand Down
1 change: 1 addition & 0 deletions cookiecutter-xblock/hooks/post_gen_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,5 @@
"setup_py_loading_pkg_data": "yes",
"setup_py_keyword_args": setup_py_keyword_args,
},
symlink_translation=True,
)
12 changes: 12 additions & 0 deletions cookiecutter-xblock/hooks/pre_gen_project.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
"""
Verify that project info is valid.
"""
import sys

if '{{ cookiecutter.package_name }}' == 'xblock': # pylint: disable=comparison-of-constants
print('ERROR: xblock is not a valid Python module name!')
sys.exit(1)

if '{{ cookiecutter.i18n_namespace }}' == '{{ cookiecutter.package_name }}': # pylint: disable=comparison-of-constants
print('ERROR: (i18n_namespace) cannot be the same as (package_name)!')
sys.exit(1)
24 changes: 8 additions & 16 deletions cookiecutter-xblock/{{cookiecutter.repo_name}}/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,12 @@
.PHONY: dev.clean dev.build dev.run upgrade help requirements
.PHONY: extract_translations compile_translations
.PHONY: detect_changed_source_translations dummy_translations build_dummy_translations
.PHONY: validate_translations pull_translations push_translations symlink_translations install_transifex_clients
.PHONY: validate_translations pull_translations push_translations install_transifex_clients

REPO_NAME := {{cookiecutter.repo_name}}
PACKAGE_NAME := {{cookiecutter.package_name}}
EXTRACT_DIR := $(PACKAGE_NAME)/locale/en/LC_MESSAGES
EXTRACTED_DJANGO := $(EXTRACT_DIR)/django-partial.po
EXTRACTED_DJANGOJS := $(EXTRACT_DIR)/djangojs-partial.po
EXTRACTED_TEXT := $(EXTRACT_DIR)/text.po
JS_TARGET := public/js/translations
TRANSLATIONS_DIR := $(PACKAGE_NAME)/translations
EXTRACT_DIR := $(PACKAGE_NAME)/conf/locale/en/LC_MESSAGES
JS_TARGET := $(PACKAGE_NAME)/public/js/translations

help:
@perl -nle'print $& if m{^[\.a-zA-Z_-]+:.*?## .*$$}' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m %-25s\033[0m %s\n", $$1, $$2}'
Expand Down Expand Up @@ -57,14 +53,13 @@ dev.run: dev.clean dev.build ## Clean, build and run test image

## Localization targets

extract_translations: symlink_translations ## extract strings to be translated, outputting .po files
cd $(PACKAGE_NAME) && i18n_tool extract
mv $(EXTRACTED_DJANGO) $(EXTRACTED_TEXT)
if [ -f "$(EXTRACTED_DJANGOJS)" ]; then cat $(EXTRACTED_DJANGOJS) >> $(EXTRACTED_TEXT); rm $(EXTRACTED_DJANGOJS); fi
extract_translations: ## extract strings to be translated, outputting .po files
cd $(PACKAGE_NAME) && i18n_tool extract --no-segment --merge-po-files
mv $(EXTRACT_DIR)/django.po $(EXTRACT_DIR)/text.po

compile_translations: symlink_translations ## compile translation files, outputting .mo files for each supported language
compile_translations: ## compile translation files, outputting .mo files for each supported language
cd $(PACKAGE_NAME) && i18n_tool generate
python manage.py compilejsi18n --namespace $(PACKAGE_NAME)i18n --output $(JS_TARGET)
python manage.py compilejsi18n --namespace {{cookiecutter.i18n_namespace}} --output $(JS_TARGET)

detect_changed_source_translations:
cd $(PACKAGE_NAME) && i18n_tool changed
Expand All @@ -82,9 +77,6 @@ pull_translations: ## pull translations from transifex
push_translations: extract_translations ## push translations to transifex
cd $(PACKAGE_NAME) && i18n_tool transifex push

symlink_translations:
if [ ! -d "$(TRANSLATIONS_DIR)" ]; then ln -s locale/ $(TRANSLATIONS_DIR); fi

install_transifex_client: ## Install the Transifex client
# Instaling client will skip CHANGELOG and LICENSE files from git changes
# so remind the user to commit the change first before installing client.
Expand Down
2 changes: 1 addition & 1 deletion cookiecutter-xblock/{{cookiecutter.repo_name}}/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ These catalogs can be created by running::
$ make extract_translations

The previous command will create the necessary ``.po`` files under
``{{cookiecutter.repo_name}}/{{cookiecutter.package_name}}/locale/en/LC_MESSAGES/text.po``.
``{{cookiecutter.repo_name}}/{{cookiecutter.package_name}}/conf/locale/en/LC_MESSAGES/text.po``.
The ``text.po`` file is created from the ``django-partial.po`` file created by
``django-admin makemessages`` (`makemessages documentation <https://docs.djangoproject.com/en/3.2/topics/i18n/translation/#message-files>`_),
this is why you will not see a ``django-partial.po`` file.
Expand Down
2 changes: 1 addition & 1 deletion cookiecutter-xblock/{{cookiecutter.repo_name}}/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
if __name__ == "__main__":
os.environ.setdefault(
"DJANGO_SETTINGS_MODULE",
"{{cookiecutter.package_name}}.locale.settings"
"translation_settings"
)

execute_from_command_line(sys.argv)
6 changes: 3 additions & 3 deletions cookiecutter-xblock/{{cookiecutter.repo_name}}/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ ignore = D101,D200,D203,D212,D215,D404,D405,D406,D407,D408,D409,D410,D411,D412,D
match-dir = (?!migrations)

[pytest]
DJANGO_SETTINGS_MODULE = {{ cookiecutter.package_name }}.settings.test
DJANGO_SETTINGS_MODULE = translation_settings
addopts = --cov {{ cookiecutter.package_name }} --cov-report term-missing --cov-report xml
norecursedirs = .* docs requirements site-packages

Expand All @@ -45,7 +45,7 @@ commands =

[testenv:docs]
setenv =
DJANGO_SETTINGS_MODULE = {{ cookiecutter.package_name }}.settings.test
DJANGO_SETTINGS_MODULE = translation_settings
PYTHONPATH = {toxinidir}
# Adding the option here instead of as a default in the docs Makefile because that Makefile is generated by shpinx.
SPHINXOPTS = -W
Expand Down Expand Up @@ -83,7 +83,7 @@ commands =

[testenv:pii_check]
setenv =
DJANGO_SETTINGS_MODULE = {{ cookiecutter.package_name }}.settings.test
DJANGO_SETTINGS_MODULE = translation_settings
deps =
-r{toxinidir}/requirements/test.txt
commands =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
"""
Django settings for {{cookiecutter.package_name}} project.
Django settings for {{cookiecutter.package_name}} project to be used in translation commands.

For more information on this file, see
https://docs.djangoproject.com/en/3.2/topics/settings/
For the full list of settings and their values, see
https://docs.djangoproject.com/en/3.2/ref/settings/
"""
import os

BASE_DIR = os.path.dirname(__file__)

SECRET_KEY = os.getenv('DJANGO_SECRET', 'open_secret')

# Application definition
Expand Down Expand Up @@ -38,9 +41,12 @@
# statici18n
# https://django-statici18n.readthedocs.io/en/latest/settings.html

LOCALE_PATHS = [os.path.join(BASE_DIR, '{{cookiecutter.package_name}}', 'conf', 'locale')]

STATICI18N_DOMAIN = 'text'
STATICI18N_NAMESPACE = '{{cookiecutter.i18n_namespace}}'
STATICI18N_PACKAGES = (
'{{cookiecutter.package_name}}.translations',
'{{cookiecutter.package_name}}',
)
STATICI18N_ROOT = '{{cookiecutter.package_name}}/public/js'
STATICI18N_OUTPUT_DIR = 'translations'
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,10 @@ function {{cookiecutter.class_name}}(runtime, element) {
*/

/* Here's where you'd do things on page load. */

// dummy_text is to have at least one string to translate in JS files. If you remove this line,
// and you don't have any other string to translate in JS files; then you must remove the (--merge-po-files)
// option from the "extract_translations" command in the Makefile
const dummy_text = gettext("Hello World");
});
}
9 changes: 8 additions & 1 deletion lib/src/edx_cookiecutter_lib/post_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def move(src, dest):
EDX_COOKIECUTTER_ROOTDIR = os.getenv('EDX_COOKIECUTTER_ROOTDIR') or 'https://github.com/openedx/edx-cookiecutters.git'


def post_gen_project(extra_context):
def post_gen_project(extra_context, symlink_translation=False):
"""
Most of what's needed after generating a project.

Expand Down Expand Up @@ -71,3 +71,10 @@ def post_gen_project(extra_context):
print(f"Since your repo will be in the {org} organization, you may need")
print("to adjust the contents of the repo, such as licenses, email addresses,")
print("and contribution details. Check with your organization.")

if symlink_translation:
package_name = extra_context["sub_dir_name"]
source_path = os.path.join(project_root_dir, package_name, "conf", "locale")
target_path = os.path.join(project_root_dir, package_name, "translation")

os.symlink(source_path, target_path)
5 changes: 3 additions & 2 deletions tests/test_cookiecutter_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,16 @@ def test_readme(options_baked, custom_template):
def test_manifest(options_baked):
"""The generated MANIFEST.in should pass a sanity check."""
manifest_text = Path("MANIFEST.in").read_text()
assert 'recursive-include myxblock *.html' in manifest_text
assert 'recursive-include my_xblock *.html' in manifest_text


def test_setup_py(options_baked):
"""The generated setup.py should pass a sanity check."""
setup_text = Path("setup.py").read_text()
assert "VERSION = get_version('myxblock', '__init__.py')" in setup_text
assert "VERSION = get_version('my_xblock', '__init__.py')" in setup_text
assert " author='Cookie Monster'," in setup_text
assert " author_email='[email protected]'," in setup_text
assert " 'my_xblock = my_xblock:MyXBlock'," in setup_text


def test_quality(options_baked):
Expand Down